From ddef3cadbab3387f3095cbcd4ce8c662c8154f2d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 17 Feb 2026 01:48:52 +0100 Subject: [PATCH] refactor: replace memory manager prototype mixing --- AGENTS.md | 2 + src/config/redact-snapshot.ts | 3 +- src/memory/manager-embedding-ops.ts | 89 ++++------------------------ src/memory/manager-sync-ops.ts | 22 ++++--- src/memory/manager.ts | 90 ++++++++++++----------------- 5 files changed, 61 insertions(+), 145 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 809daf014a..9c825792b0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,8 @@ - Language: TypeScript (ESM). Prefer strict typing; avoid `any`. - Formatting/linting via Oxlint and Oxfmt; run `pnpm check` before commits. - Never add `@ts-nocheck` and do not disable `no-explicit-any`; fix root causes and update Oxlint/Oxfmt config only when required. +- Never share class behavior via prototype mutation (`applyPrototypeMixins`, `Object.defineProperty` on `.prototype`, or exporting `Class.prototype` for merges). Use explicit inheritance/composition (`A extends B extends C`) or helper composition so TypeScript can typecheck. +- If this pattern is needed, stop and get explicit approval before shipping; default behavior is to split/refactor into an explicit class hierarchy and keep members strongly typed. - Add brief code comments for tricky or non-obvious logic. - Keep files concise; extract helpers instead of “V2” copies. Use existing patterns for CLI options and dependency injection via `createDefaultDeps`. - Aim to keep files under ~700 LOC; guideline only (not a hard guardrail). Split/refactor when it improves clarity or testability. diff --git a/src/config/redact-snapshot.ts b/src/config/redact-snapshot.ts index ddf9be2865..81b02b8225 100644 --- a/src/config/redact-snapshot.ts +++ b/src/config/redact-snapshot.ts @@ -1,6 +1,6 @@ +import type { ConfigFileSnapshot } from "./types.openclaw.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { isSensitiveConfigPath, type ConfigUiHints } from "./schema.hints.js"; -import type { ConfigFileSnapshot } from "./types.openclaw.js"; const log = createSubsystemLogger("config/redaction"); const ENV_VAR_PLACEHOLDER_PATTERN = /^\$\{[^}]*\}$/; @@ -369,7 +369,6 @@ class RedactionError extends Error { super("internal error class---should never escape"); this.key = key; this.name = "RedactionError"; - Object.setPrototypeOf(this, RedactionError.prototype); } } diff --git a/src/memory/manager-embedding-ops.ts b/src/memory/manager-embedding-ops.ts index ac241556be..849014feda 100644 --- a/src/memory/manager-embedding-ops.ts +++ b/src/memory/manager-embedding-ops.ts @@ -1,7 +1,6 @@ import fs from "node:fs/promises"; -import type { DatabaseSync } from "node:sqlite"; -import { type FSWatcher } from "chokidar"; -import type { ResolvedMemorySearchConfig } from "../agents/memory-search.js"; +import type { SessionFileEntry } from "./session-files.js"; +import type { MemorySource } from "./types.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { runGeminiEmbeddingBatches, type GeminiBatchRequest } from "./batch-gemini.js"; import { @@ -12,12 +11,6 @@ import { import { type VoyageBatchRequest, runVoyageEmbeddingBatches } from "./batch-voyage.js"; import { enforceEmbeddingMaxInputTokens } from "./embedding-chunk-limits.js"; import { estimateUtf8Bytes } from "./embedding-input-limits.js"; -import { - type EmbeddingProvider, - type GeminiEmbeddingClient, - type OpenAiEmbeddingClient, - type VoyageEmbeddingClient, -} from "./embeddings.js"; import { chunkMarkdown, hashText, @@ -26,8 +19,7 @@ import { type MemoryChunk, type MemoryFileEntry, } from "./internal.js"; -import type { SessionFileEntry } from "./session-files.js"; -import type { MemorySource } from "./types.js"; +import { MemoryManagerSyncOps } from "./manager-sync-ops.js"; const VECTOR_TABLE = "chunks_vec"; const FTS_TABLE = "chunks_fts"; @@ -48,62 +40,7 @@ const vectorToBlob = (embedding: number[]): Buffer => const log = createSubsystemLogger("memory"); -abstract class MemoryManagerEmbeddingOps { - protected abstract readonly agentId: string; - protected abstract readonly workspaceDir: string; - protected abstract readonly settings: ResolvedMemorySearchConfig; - protected provider: EmbeddingProvider | null = null; - protected fallbackFrom?: "openai" | "local" | "gemini" | "voyage"; - protected openAi?: OpenAiEmbeddingClient; - protected gemini?: GeminiEmbeddingClient; - protected voyage?: VoyageEmbeddingClient; - protected abstract batch: { - enabled: boolean; - wait: boolean; - concurrency: number; - pollIntervalMs: number; - timeoutMs: number; - }; - protected readonly sources: Set = new Set(); - protected providerKey: string | null = null; - protected abstract readonly vector: { - enabled: boolean; - available: boolean | null; - extensionPath?: string; - loadError?: string; - dims?: number; - }; - protected readonly fts: { - enabled: boolean; - available: boolean; - loadError?: string; - } = { enabled: false, available: false }; - protected vectorReady: Promise | null = null; - protected watcher: FSWatcher | null = null; - protected watchTimer: NodeJS.Timeout | null = null; - protected sessionWatchTimer: NodeJS.Timeout | null = null; - protected sessionUnsubscribe: (() => void) | null = null; - protected fallbackReason?: string; - protected intervalTimer: NodeJS.Timeout | null = null; - protected closed = false; - protected dirty = false; - protected sessionsDirty = false; - protected sessionsDirtyFiles = new Set(); - protected sessionPendingFiles = new Set(); - protected sessionDeltas = new Map< - string, - { lastSize: number; pendingBytes: number; pendingMessages: number } - >(); - - protected batchFailureCount = 0; - protected batchFailureLastError?: string; - protected batchFailureLastProvider?: string; - protected batchFailureLock: Promise = Promise.resolve(); - - protected abstract readonly cache: { enabled: boolean; maxEntries?: number }; - protected abstract db: DatabaseSync; - protected abstract ensureVectorReady(dimensions?: number): Promise; - +export class MemoryManagerEmbeddingOps extends MemoryManagerSyncOps { private buildEmbeddingBatches(chunks: MemoryChunk[]): MemoryChunk[][] { const batches: MemoryChunk[][] = []; let current: MemoryChunk[] = []; @@ -204,7 +141,7 @@ abstract class MemoryManagerEmbeddingOps { } } - private pruneEmbeddingCacheIfNeeded(): void { + protected pruneEmbeddingCacheIfNeeded(): void { if (!this.cache.enabled) { return; } @@ -400,13 +337,13 @@ abstract class MemoryManagerEmbeddingOps { chunks: MemoryChunk[]; source: MemorySource; }): { - agentId: string; + agentId: string | undefined; requests: TRequest[]; wait: boolean; concurrency: number; pollIntervalMs: number; timeoutMs: number; - debug: (message: string, data?: Record) => void; + debug: (message: string, data: Record) => void; } { const { requests, chunks, source } = params; return { @@ -553,7 +490,7 @@ abstract class MemoryManagerEmbeddingOps { return embeddings; } - private async embedBatchWithRetry(texts: string[]): Promise { + protected async embedBatchWithRetry(texts: string[]): Promise { if (texts.length === 0) { return []; } @@ -606,7 +543,7 @@ abstract class MemoryManagerEmbeddingOps { return isLocal ? EMBEDDING_BATCH_TIMEOUT_LOCAL_MS : EMBEDDING_BATCH_TIMEOUT_REMOTE_MS; } - private async embedQueryWithTimeout(text: string): Promise { + protected async embedQueryWithTimeout(text: string): Promise { if (!this.provider) { throw new Error("Cannot embed query in FTS-only mode (no embedding provider)"); } @@ -619,7 +556,7 @@ abstract class MemoryManagerEmbeddingOps { ); } - private async withTimeout( + protected async withTimeout( promise: Promise, timeoutMs: number, message: string, @@ -747,11 +684,11 @@ abstract class MemoryManagerEmbeddingOps { } } - private getIndexConcurrency(): number { + protected getIndexConcurrency(): number { return this.batch.enabled ? this.batch.concurrency : EMBEDDING_INDEX_CONCURRENCY; } - private async indexFile( + protected async indexFile( entry: MemoryFileEntry | SessionFileEntry, options: { source: MemorySource; content?: string }, ) { @@ -865,5 +802,3 @@ abstract class MemoryManagerEmbeddingOps { .run(entry.path, options.source, entry.hash, entry.mtimeMs, entry.size); } } - -export const memoryManagerEmbeddingOps = MemoryManagerEmbeddingOps.prototype; diff --git a/src/memory/manager-sync-ops.ts b/src/memory/manager-sync-ops.ts index 831255f419..005eed31e5 100644 --- a/src/memory/manager-sync-ops.ts +++ b/src/memory/manager-sync-ops.ts @@ -1,9 +1,11 @@ +import type { DatabaseSync } from "node:sqlite"; +import chokidar, { FSWatcher } from "chokidar"; import { randomUUID } from "node:crypto"; import fsSync from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; -import type { DatabaseSync } from "node:sqlite"; -import chokidar, { FSWatcher } from "chokidar"; +import type { SessionFileEntry } from "./session-files.js"; +import type { MemorySource, MemorySyncProgressUpdate } from "./types.js"; import { resolveAgentDir } from "../agents/agent-scope.js"; import { ResolvedMemorySearchConfig } from "../agents/memory-search.js"; import { type OpenClawConfig } from "../config/config.js"; @@ -35,10 +37,8 @@ import { listSessionFilesForAgent, sessionPathForFile, } from "./session-files.js"; -import type { SessionFileEntry } from "./session-files.js"; import { loadSqliteVecExtension } from "./sqlite-vec.js"; import { requireNodeSqlite } from "./sqlite.js"; -import type { MemorySource, MemorySyncProgressUpdate } from "./types.js"; type MemoryIndexMeta = { model: string; @@ -81,7 +81,7 @@ function shouldIgnoreMemoryWatchPath(watchPath: string): boolean { return parts.some((segment) => IGNORED_MEMORY_WATCH_DIR_NAMES.has(segment)); } -abstract class MemoryManagerSyncOps { +export abstract class MemoryManagerSyncOps { protected abstract readonly cfg: OpenClawConfig; protected abstract readonly agentId: string; protected abstract readonly workspaceDir: string; @@ -149,7 +149,7 @@ abstract class MemoryManagerSyncOps { options: { source: MemorySource; content?: string }, ): Promise; - private async ensureVectorReady(dimensions?: number): Promise { + protected async ensureVectorReady(dimensions?: number): Promise { if (!this.vector.enabled) { return false; } @@ -334,7 +334,7 @@ abstract class MemoryManagerSyncOps { await Promise.all(suffixes.map((suffix) => fs.rm(`${basePath}${suffix}`, { force: true }))); } - private ensureSchema() { + protected ensureSchema() { const result = ensureMemoryIndexSchema({ db: this.db, embeddingCacheTable: EMBEDDING_CACHE_TABLE, @@ -910,7 +910,7 @@ abstract class MemoryManagerSyncOps { return /embedding|embeddings|batch/i.test(message); } - private resolveBatchConfig(): { + protected resolveBatchConfig(): { enabled: boolean; wait: boolean; concurrency: number; @@ -1141,7 +1141,7 @@ abstract class MemoryManagerSyncOps { this.sessionsDirtyFiles.clear(); } - private readMeta(): MemoryIndexMeta | null { + protected readMeta(): MemoryIndexMeta | null { const row = this.db.prepare(`SELECT value FROM meta WHERE key = ?`).get(META_KEY) as | { value: string } | undefined; @@ -1155,7 +1155,7 @@ abstract class MemoryManagerSyncOps { } } - private writeMeta(meta: MemoryIndexMeta) { + protected writeMeta(meta: MemoryIndexMeta) { const value = JSON.stringify(meta); this.db .prepare( @@ -1164,5 +1164,3 @@ abstract class MemoryManagerSyncOps { .run(META_KEY, value); } } - -export const memoryManagerSyncOps = MemoryManagerSyncOps.prototype; diff --git a/src/memory/manager.ts b/src/memory/manager.ts index 2e28a07b48..88253c085f 100644 --- a/src/memory/manager.ts +++ b/src/memory/manager.ts @@ -1,11 +1,19 @@ -import fs from "node:fs/promises"; -import path from "node:path"; import type { DatabaseSync } from "node:sqlite"; import { type FSWatcher } from "chokidar"; -import { resolveAgentDir, resolveAgentWorkspaceDir } from "../agents/agent-scope.js"; +import fs from "node:fs/promises"; +import path from "node:path"; import type { ResolvedMemorySearchConfig } from "../agents/memory-search.js"; -import { resolveMemorySearchConfig } from "../agents/memory-search.js"; import type { OpenClawConfig } from "../config/config.js"; +import type { + MemoryEmbeddingProbeResult, + MemoryProviderStatus, + MemorySearchManager, + MemorySearchResult, + MemorySource, + MemorySyncProgressUpdate, +} from "./types.js"; +import { resolveAgentDir, resolveAgentWorkspaceDir } from "../agents/agent-scope.js"; +import { resolveMemorySearchConfig } from "../agents/memory-search.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { createEmbeddingProvider, @@ -17,18 +25,9 @@ import { } from "./embeddings.js"; import { bm25RankToScore, buildFtsQuery, mergeHybridResults } from "./hybrid.js"; import { isMemoryPath, normalizeExtraMemoryPaths } from "./internal.js"; -import { memoryManagerEmbeddingOps } from "./manager-embedding-ops.js"; +import { MemoryManagerEmbeddingOps } from "./manager-embedding-ops.js"; import { searchKeyword, searchVector } from "./manager-search.js"; -import { memoryManagerSyncOps } from "./manager-sync-ops.js"; import { extractKeywords } from "./query-expansion.js"; -import type { - MemoryEmbeddingProbeResult, - MemoryProviderStatus, - MemorySearchManager, - MemorySearchResult, - MemorySource, - MemorySyncProgressUpdate, -} from "./types.js"; const SNIPPET_MAX_CHARS = 700; const VECTOR_TABLE = "chunks_vec"; const FTS_TABLE = "chunks_fts"; @@ -39,23 +38,21 @@ const log = createSubsystemLogger("memory"); const INDEX_CACHE = new Map(); -export class MemoryIndexManager implements MemorySearchManager { - // oxlint-disable-next-line typescript/no-explicit-any - [key: string]: any; +export class MemoryIndexManager extends MemoryManagerEmbeddingOps implements MemorySearchManager { private readonly cacheKey: string; protected readonly cfg: OpenClawConfig; protected readonly agentId: string; - private readonly workspaceDir: string; - private readonly settings: ResolvedMemorySearchConfig; - private provider: EmbeddingProvider | null; + protected readonly workspaceDir: string; + protected readonly settings: ResolvedMemorySearchConfig; + protected provider: EmbeddingProvider | null; private readonly requestedProvider: "openai" | "local" | "gemini" | "voyage" | "auto"; - private fallbackFrom?: "openai" | "local" | "gemini" | "voyage"; - private fallbackReason?: string; + protected fallbackFrom?: "openai" | "local" | "gemini" | "voyage"; + protected fallbackReason?: string; private readonly providerUnavailableReason?: string; protected openAi?: OpenAiEmbeddingClient; protected gemini?: GeminiEmbeddingClient; protected voyage?: VoyageEmbeddingClient; - private batch: { + protected batch: { enabled: boolean; wait: boolean; concurrency: number; @@ -65,31 +62,32 @@ export class MemoryIndexManager implements MemorySearchManager { protected batchFailureCount = 0; protected batchFailureLastError?: string; protected batchFailureLastProvider?: string; - private db: DatabaseSync; - private readonly sources: Set; + protected batchFailureLock: Promise = Promise.resolve(); + protected db: DatabaseSync; + protected readonly sources: Set; protected providerKey: string; - private readonly cache: { enabled: boolean; maxEntries?: number }; - private readonly vector: { + protected readonly cache: { enabled: boolean; maxEntries?: number }; + protected readonly vector: { enabled: boolean; available: boolean | null; extensionPath?: string; loadError?: string; dims?: number; }; - private readonly fts: { + protected readonly fts: { enabled: boolean; available: boolean; loadError?: string; }; protected vectorReady: Promise | null = null; - private watcher: FSWatcher | null = null; - private watchTimer: NodeJS.Timeout | null = null; - private sessionWatchTimer: NodeJS.Timeout | null = null; - private sessionUnsubscribe: (() => void) | null = null; - private intervalTimer: NodeJS.Timeout | null = null; - private closed = false; - private dirty = false; - private sessionsDirty = false; + protected watcher: FSWatcher | null = null; + protected watchTimer: NodeJS.Timeout | null = null; + protected sessionWatchTimer: NodeJS.Timeout | null = null; + protected sessionUnsubscribe: (() => void) | null = null; + protected intervalTimer: NodeJS.Timeout | null = null; + protected closed = false; + protected dirty = false; + protected sessionsDirty = false; protected sessionsDirtyFiles = new Set(); protected sessionPendingFiles = new Set(); protected sessionDeltas = new Map< @@ -146,6 +144,7 @@ export class MemoryIndexManager implements MemorySearchManager { providerResult: EmbeddingProviderResult; purpose?: "default" | "status"; }) { + super(); this.cacheKey = params.cacheKey; this.cfg = params.cfg; this.agentId = params.agentId; @@ -267,7 +266,7 @@ export class MemoryIndexManager implements MemorySearchManager { ? await this.searchKeyword(cleaned, candidates).catch(() => []) : []; - const queryVec = (await this.embedQueryWithTimeout(cleaned)) as number[]; + const queryVec = await this.embedQueryWithTimeout(cleaned); const hasVector = queryVec.some((v) => v !== 0); const vectorResults = hasVector ? await this.searchVector(queryVec, candidates).catch(() => []) @@ -618,20 +617,3 @@ export class MemoryIndexManager implements MemorySearchManager { INDEX_CACHE.delete(this.cacheKey); } } - -function applyPrototypeMixins(target: object, ...sources: object[]): void { - for (const source of sources) { - for (const name of Object.getOwnPropertyNames(source)) { - if (name === "constructor") { - continue; - } - const descriptor = Object.getOwnPropertyDescriptor(source, name); - if (!descriptor) { - continue; - } - Object.defineProperty(target, name, descriptor); - } - } -} - -applyPrototypeMixins(MemoryIndexManager.prototype, memoryManagerSyncOps, memoryManagerEmbeddingOps);