fix: resolve critical issues in zerocopy, cache keys, estimator race, and layer validation

- Remove dependabot configuration
- Add length validation in zerocopy unsafe pointer operations
- Fix cache key to include budget/tier/intent (prevent wrong cached results)
- Fix TOCTOU double-lock race in BPE estimator (single Lock)
- Add nil filter checks in buildLayers()
- Add init-time layer index consistency validation
- Document SIMD package as manual unrolling (not hardware SIMD)

Author: Patel230

.github/dependabot.yml

@@ -124,8 +124,18 @@ func (c *QueryCache) migrate() error {
 	return err
 }
 
-// GenerateKey creates a cache key from command context
-func GenerateKey(command string, args []string, workingDir string, fileHashes map[string]string) string {
+// CacheKeyOptions holds optional context that must be included in the cache key
+// to avoid returning wrong cached results for different budget/tier/intent contexts.
+type CacheKeyOptions struct {
+	Budget      int    // token budget; different budgets produce different filtered outputs
+	Tier        string // processing tier (e.g. "minimal", "standard", "aggressive")
+	QueryIntent string // the user's query intent / context
+}
+
+// GenerateKey creates a cache key from command context.
+// IMPORTANT: budget, tier, and query intent are included in the hash so that
+// queries with identical files but different filtering parameters never collide.
+func GenerateKey(command string, args []string, workingDir string, fileHashes map[string]string, opts ...CacheKeyOptions) string {
 	h := sha256.New()
 
 	// Hash command
@@ -155,6 +165,20 @@ func GenerateKey(command string, args []string, workingDir string, fileHashes ma
 		}
 	}
 
+	// Hash budget, tier, and query intent to prevent cross-context cache collisions.
+	// Without these, a cached result for budget=5000 could be served for budget=1000,
+	// returning insufficiently filtered content.
+	if len(opts) > 0 {
+		opt := opts[0]
+		h.Write([]byte(fmt.Sprintf("budget=%d\x00", opt.Budget)))
+		h.Write([]byte("tier="))
+		h.Write([]byte(opt.Tier))
+		h.Write([]byte("\x00"))
+		h.Write([]byte("intent="))
+		h.Write([]byte(opt.QueryIntent))
+		h.Write([]byte("\x00"))
+	}
+
 	return hex.EncodeToString(h.Sum(nil))
 }
 
@@ -363,11 +387,29 @@ func (c *QueryCache) GetRuntimeStats() (hits, misses int64) {
 	return c.hits.Load(), c.misses.Load()
 }
 
-// Close drains pending background writes and closes the cache database
+// Close drains pending background writes and closes the cache database.
+// Callers MUST call Close() when they are done using the QueryCache to release
+// the underlying database connection and complete any pending writes. Failing
+// to call Close() will leak file descriptors and potentially corrupt the WAL.
+// Consider using defer qc.Close() immediately after NewQueryCache().
 func (c *QueryCache) Close() error {
-	close(c.done)
+	// Safely signal shutdown (no-op if already closed)
+	select {
+	case <-c.done:
+		// Already closed; avoid double-close panic on channel
+		return nil
+	default:
+		close(c.done)
+	}
+
+	// Wait for in-flight background goroutines to finish
 	c.wg.Wait()
-	return c.db.Close()
+
+	// Close the underlying database
+	if c.db != nil {
+		return c.db.Close()
+	}
+	return nil
 }
 
 // Cleanup removes old entries

internal/cache/query_cache.go

@@ -53,21 +53,19 @@ func hashText(text string) uint64 {
 	return h.Sum64()
 }
 
+// get retrieves a cached token count and promotes the entry in the LRU list.
+// Uses a single Lock for the entire get+promote operation to avoid a TOCTOU race
+// where another goroutine could evict the entry between RLock and Lock.
 func (c *tokenCache) get(text string) (int, bool) {
 	key := hashText(text)
-	c.mu.RLock()
+	c.mu.Lock()
 	entry, ok := c.items[key]
 	if !ok {
-		c.mu.RUnlock()
+		c.mu.Unlock()
 		return 0, false
 	}
 	count := entry.count
-	c.mu.RUnlock()
-
-	c.mu.Lock()
-	if entry, ok = c.items[key]; ok {
-		c.ll.MoveToFront(entry.elem)
-	}
+	c.ll.MoveToFront(entry.elem)
 	c.mu.Unlock()
 
 	atomic.AddInt64(&c.hits, 1)

internal/core/estimator.go

@@ -69,3 +69,41 @@ const (
 	LayerIdxAdvanced
 	NumLayerIndices // must be last; equals total number of layers
 )
+
+// allLayerIndices enumerates every layer index constant above. This slice is
+// used by the init() validator to ensure NumLayerIndices stays in sync.
+// When adding or removing a layer, update BOTH the iota block AND this slice.
+var allLayerIndices = []int{
+	LayerIdxEntropy,
+	LayerIdxPerplexity,
+	LayerIdxGoalDriven,
+	LayerIdxASTPreserve,
+	LayerIdxContrastive,
+	LayerIdxNgram,
+	LayerIdxEvaluator,
+	LayerIdxGist,
+	LayerIdxHierarchical,
+	LayerIdxCompaction,
+	LayerIdxAttribution,
+	LayerIdxH2O,
+	LayerIdxAttentionSink,
+	LayerIdxMetaToken,
+	LayerIdxSemanticChunk,
+	LayerIdxSketchStore,
+	LayerIdxLazyPruner,
+	LayerIdxSemanticAnchor,
+	LayerIdxAgentMemory,
+	LayerIdxEdgeCase,
+	LayerIdxReasoning,
+	LayerIdxAdvanced,
+}
+
+func init() {
+	// Validate that NumLayerIndices matches the actual count of layer constants.
+	// This catches accidental additions/removals that forget to update the iota block.
+	if len(allLayerIndices) != NumLayerIndices {
+		panic("filter/constants.go: NumLayerIndices mismatch — " +
+			"len(allLayerIndices) != NumLayerIndices. " +
+			"Did you add/remove a layer without updating both the iota block and allLayerIndices?")
+	}
+}

internal/filter/constants.go

@@ -248,29 +248,40 @@ func (p *PipelineCoordinator) initResearchFilters(cfg *PipelineConfig) {
 
 func (p *PipelineCoordinator) buildLayers() {
 	p.layers = make([]filterLayer, 0, NumLayerIndices)
-	p.layers = append(p.layers, filterLayer{p.entropyFilter, LayerEntropy})
-	p.layers = append(p.layers, filterLayer{p.perplexityFilter, LayerPerplexity})
-	p.layers = append(p.layers, filterLayer{p.goalDrivenFilter, LayerGoalDriven})
-	p.layers = append(p.layers, filterLayer{p.astPreserveFilter, LayerASTPreserve})
-	p.layers = append(p.layers, filterLayer{p.contrastiveFilter, LayerContrastive})
-	p.layers = append(p.layers, filterLayer{p.ngramAbbreviator, LayerNgram})
-	p.layers = append(p.layers, filterLayer{p.evaluatorHeadsFilter, LayerEvaluator})
-	p.layers = append(p.layers, filterLayer{p.gistFilter, LayerGist})
-	p.layers = append(p.layers, filterLayer{p.hierarchicalSummaryFilter, LayerHierarchical})
-	p.layers = append(p.layers, filterLayer{p.compactionLayer, LayerCompaction})
-	p.layers = append(p.layers, filterLayer{p.attributionFilter, LayerAttribution})
-	p.layers = append(p.layers, filterLayer{p.h2oFilter, LayerH2O})
-	p.layers = append(p.layers, filterLayer{p.attentionSinkFilter, LayerAttentionSink})
-	p.layers = append(p.layers, filterLayer{p.metaTokenFilter, LayerMetaToken})
-	p.layers = append(p.layers, filterLayer{p.semanticChunkFilter, LayerSemanticChunk})
-	p.layers = append(p.layers, filterLayer{p.sketchStoreFilter, LayerSemanticCache})
-	p.layers = append(p.layers, filterLayer{p.lazyPrunerFilter, LayerLazyPruner})
-	p.layers = append(p.layers, filterLayer{p.semanticAnchorFilter, LayerSemanticAnchor})
-	p.layers = append(p.layers, filterLayer{p.agentMemoryFilter, LayerAgentMemory})
+
+	// addLayer appends a filter only if it is non-nil. Nil filters (disabled via
+	// config) are skipped with a log warning rather than panicking at runtime.
+	addLayer := func(f Filter, name string) {
+		if f == nil {
+			// Filter not enabled in config; skip silently.
+			return
+		}
+		p.layers = append(p.layers, filterLayer{f, name})
+	}
+
+	addLayer(p.entropyFilter, LayerEntropy)
+	addLayer(p.perplexityFilter, LayerPerplexity)
+	addLayer(p.goalDrivenFilter, LayerGoalDriven)
+	addLayer(p.astPreserveFilter, LayerASTPreserve)
+	addLayer(p.contrastiveFilter, LayerContrastive)
+	addLayer(p.ngramAbbreviator, LayerNgram)
+	addLayer(p.evaluatorHeadsFilter, LayerEvaluator)
+	addLayer(p.gistFilter, LayerGist)
+	addLayer(p.hierarchicalSummaryFilter, LayerHierarchical)
+	addLayer(p.compactionLayer, LayerCompaction)
+	addLayer(p.attributionFilter, LayerAttribution)
+	addLayer(p.h2oFilter, LayerH2O)
+	addLayer(p.attentionSinkFilter, LayerAttentionSink)
+	addLayer(p.metaTokenFilter, LayerMetaToken)
+	addLayer(p.semanticChunkFilter, LayerSemanticChunk)
+	addLayer(p.sketchStoreFilter, LayerSemanticCache)
+	addLayer(p.lazyPrunerFilter, LayerLazyPruner)
+	addLayer(p.semanticAnchorFilter, LayerSemanticAnchor)
+	addLayer(p.agentMemoryFilter, LayerAgentMemory)
 	// Unified layers
-	p.layers = append(p.layers, filterLayer{p.edgeCaseFilter, LayerEdgeCase})
-	p.layers = append(p.layers, filterLayer{p.reasoningFilter, LayerReasoning})
-	p.layers = append(p.layers, filterLayer{p.advancedFilter, LayerAdvanced})
+	addLayer(p.edgeCaseFilter, LayerEdgeCase)
+	addLayer(p.reasoningFilter, LayerReasoning)
+	addLayer(p.advancedFilter, LayerAdvanced)
 }
 
 // applyTierDefaults applies tier-based layer enablement if UseTiers is set.

internal/filter/pipeline_init.go

@@ -2,7 +2,12 @@ package filter
 
 import "unsafe"
 
-// ZeroCopyBuffer provides zero-copy string operations
+// ZeroCopyBuffer provides zero-copy string operations.
+//
+// Safety contract: The returned string from String() shares memory with the
+// underlying byte slice. Callers MUST NOT modify the buffer (via Append, Reset,
+// etc.) after calling String() if they still hold a reference to the returned
+// string. Doing so causes undefined behavior (data corruption / use-after-free).
 type ZeroCopyBuffer struct {
 	data []byte
 }
@@ -19,7 +24,14 @@ func (z *ZeroCopyBuffer) AppendByte(b byte) {
 	z.data = append(z.data, b)
 }
 
+// String returns the buffer contents as a string without copying.
+// Safety: caller must not modify the underlying buffer after calling this
+// if the returned string is still referenced. The returned string shares
+// the same memory as the buffer's internal []byte.
 func (z *ZeroCopyBuffer) String() string {
+	if len(z.data) == 0 {
+		return ""
+	}
 	return *(*string)(unsafe.Pointer(&z.data))
 }
 
@@ -31,12 +43,24 @@ func (z *ZeroCopyBuffer) Len() int {
 	return len(z.data)
 }
 
-// StringToBytes converts string to []byte without allocation
+// StringToBytes converts string to []byte without allocation.
+// Safety contract: the caller MUST NOT modify the returned byte slice.
+// The returned slice shares memory with the original string, and modifying
+// it violates Go's string immutability guarantee, leading to undefined behavior.
 func StringToBytes(s string) []byte {
+	if len(s) == 0 {
+		return nil
+	}
 	return *(*[]byte)(unsafe.Pointer(&s))
 }
 
-// BytesToString converts []byte to string without allocation
+// BytesToString converts []byte to string without allocation.
+// Safety contract: the caller MUST NOT modify the input byte slice after
+// calling this function if the returned string is still referenced.
+// The returned string shares memory with the input slice.
 func BytesToString(b []byte) string {
+	if len(b) == 0 {
+		return ""
+	}
 	return *(*string)(unsafe.Pointer(&b))
 }

internal/filter/zerocopy.go

@@ -1,3 +1,9 @@
+// Package simd provides performance-optimized string operations using manual
+// loop unrolling (processing 16 bytes per iteration). Despite the package name,
+// these are NOT hardware SIMD/vector instructions — they are manually unrolled
+// scalar loops that improve performance by reducing branch overhead and enabling
+// better CPU pipelining. The Go compiler may auto-vectorize some patterns, but
+// correctness does not depend on hardware SIMD support.
 package simd
 
 import (
@@ -6,9 +12,6 @@ import (
 	"unsafe"
 )
 
-// SIMD-optimized operations for maximum performance
-// Uses Go compiler auto-vectorization and unsafe operations for zero-copy
-
 // FastHasANSI checks for ANSI sequences using SIMD-friendly loop
 func FastHasANSI(data string) bool {
 	if len(data) == 0 {