fix: apply repository quality fixes#7
Conversation
|
@coderabbitai Please perform a comprehensive full-repo audit focusing on security, type safety, error handling, resource leaks, logic bugs, cross-platform issues, and test coverage. Review all 83 files thoroughly. |
📝 WalkthroughWalkthroughThis PR adds a single-line non-functional audit marker comment ( ChangesAudit Marker Additions
Estimated Code Review Effort🎯 1 (Trivial) | ⏱️ ~5 minutes This is a homogeneous, mechanically repetitive change across 72 files (one identical comment line per file). Despite the large file count, the pattern is uniform and cosmetic, requiring only spot-checking to verify consistency. Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Tip For best results, initiate chat on the files or code changes.
The full audit will cover all 56 source files, 22 test files, and 3 scripts across your Bun 1.x / Node.js 20+ runtime targeting Windows, Linux, and macOS. I'll post detailed findings inline on the relevant files shortly. [full_review] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (33)
src/services/web-server.ts (2)
155-165:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset
startPromiseon shutdown to allow clean restart.Line 155-165: after
stop(),startPromiseremains set, so laterstart()can short-circuit without re-binding the server.Suggested fix
async stop(): Promise<void> { this.stopHealthCheckLoop(); + this.startPromise = null; if (!this.isOwner || !this.server) { return; } this.server.stop(); this.server = null; this.isOwner = false; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/web-server.ts` around lines 155 - 165, stop() currently clears server and isOwner but leaves startPromise set, causing subsequent start() calls to short-circuit and not re-bind; update the stop() method (class method stop) to also clear/reset this.startPromise (e.g., set to null/undefined) after shutting down the server so that start() will create a new promise and perform a fresh bind on restart.
449-452:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
changelogIdquery parameter is currently broken by a typo.Line 450 reads
chlogIdwhile Line 452 expectschangelogId, so clients using the documented name fail.Suggested fix
- const changelogId = url.searchParams.get("chlogId"); + const changelogId = + url.searchParams.get("changelogId") ?? url.searchParams.get("chlogId");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/web-server.ts` around lines 449 - 452, The query parameter key is misspelled: the code reads url.searchParams.get("chlogId") but the rest of the handler uses the variable changelogId and the documented param name is "changelogId"; change the searchParams key to "changelogId" (replace "chlogId" with "changelogId") so the const changelogId = url.searchParams.get(...) call returns the expected value and the existing error branch remains correct.src/services/platform-server.ts (1)
38-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a request body size cap to prevent memory-exhaustion DoS.
Line 38-43 buffers the full body with no upper bound, so oversized requests can exhaust process memory.
Suggested fix
+ const MAX_BODY_BYTES = 1_048_576; // 1 MiB + let totalBytes = 0; const chunks: Buffer[] = []; for await (const chunk of req) { - chunks.push(chunk); + const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + totalBytes += buf.length; + if (totalBytes > MAX_BODY_BYTES) { + res.statusCode = 413; + res.end("Payload too large"); + return; + } + chunks.push(buf); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/platform-server.ts` around lines 38 - 43, The code currently streams the request into an unbounded Buffer array (chunks) and concatenates to body, which allows memory-exhaustion attacks; add a MAX_BODY_SIZE constant (e.g., bytes) and while iterating the for-await-of over req in platform-server.ts track the accumulated size, reject the request (throw an error or respond with 413 Payload Too Large and destroy the socket) as soon as the size exceeds MAX_BODY_SIZE, and only call Buffer.concat(chunks) when under the limit; update any callers of body to handle the error/413 response.src/services/ai/providers/anthropic-messages.ts (1)
173-183:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid logging raw tool payloads in validation failures.
Line 182 logs
rawDatafrom tool output; this can leak user/profile content into logs.Suggested fix
log("Anthropic tool response validation failed", { error: String(validationError), stack: errorStack, errorType: validationError instanceof Error ? validationError.constructor.name : typeof validationError, toolName: toolSchema.function.name, iteration: iterations, - rawData: JSON.stringify(toolUse).slice(0, 500), + rawDataPreview: "[REDACTED]", });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/ai/providers/anthropic-messages.ts` around lines 173 - 183, The current log in the Anthropic tool validation path writes the raw tool payload (toolUse) which can leak sensitive user/profile data; update the logging inside the validation failure block (where log(...) is called in anthropic-messages.ts) to remove rawData logging and instead log a sanitized summary: include toolSchema.function.name, iteration count (iterations), error/stack, and a safe, non-sensitive indicator such as payload size or a redacted/hashed fingerprint of toolUse (do not serialize or include user content). Ensure you reference the existing variables error/validationError, errorStack, toolSchema.function.name and iterations and replace JSON.stringify(toolUse) with the safe summary/redaction before calling log.src/services/api-handlers.ts (2)
411-443:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftDelete-before-insert update flow can permanently lose memory data.
Line 411 deletes the existing record before external embedding calls (Line 417/420) and reinsertion. Any failure in between leaves the memory removed.
Please make this atomic (transactional swap or update-in-place) and only remove old state after new vectors are safely persisted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api-handlers.ts` around lines 411 - 443, The current flow deletes the vector with vectorSearch.deleteVector(...) before computing embeddings and reinserting, so failures (embeddingService.embedWithTimeout or insertVector) can permanently lose data; change to compute new embeddings first (call embeddingService.embedWithTimeout on newContent and tags), then insert the new vector (vectorSearch.insertVector) into the same shard/db (or use an update-in-place API if available), and only upon successful insert delete the old vector and call shardManager.decrementVectorCount(foundShard.id) / incrementVectorCount(foundShard.id) as appropriate; ensure operations use the same foundShard and db and handle errors by rolling back the new insert if delete fails or by avoiding delete altogether via an atomic update method.
1091-1097:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
resolvedfilter is ignored in conflict listing.Line 1092 accepts
resolved, but Line 1096 always returns unresolved conflicts, so API behavior is misleading.Either apply the
resolvedflag in data retrieval or reject unsupported values explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api-handlers.ts` around lines 1091 - 1097, handleListConflicts currently ignores the resolved parameter by always calling getAllUnresolvedConflicts; update the handler to honor the resolved flag (either call an existing getAllConflicts/getConflicts API with a resolved parameter or extend getAllUnresolvedConflicts to accept a resolved boolean) and map the returned list the same way, or explicitly validate and reject unsupported resolved values (throw or return 400) if you intend not to support filtering; ensure changes reference handleListConflicts and getAllUnresolvedConflicts (or the replacement conflict-retrieval function) so the correct data path is used.src/services/cleanup-service.ts (2)
65-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
linkedMemoriesDeletedis never updated.
linkedMemoriesDeleted(Line 65) is returned (Line 113) but never incremented, so the reported value is always0.Also applies to: 113-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/cleanup-service.ts` around lines 65 - 66, linkedMemoriesDeleted is never incremented so the returned count is always 0; update the deletion logic inside the cleanup routine (where you iterate over memories and call delete/remove) to increment linkedMemoriesDeleted whenever a linked memory is actually deleted, and likewise ensure pinnedSkipped is incremented in the branch that skips pinned memories; look for symbols linkedMemoriesDeleted and pinnedSkipped in cleanup-service.ts and add the increments in the corresponding delete/skip branches before returning those variables.
40-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate cleanup cooldown only after successful completion.
At Line 41,
lastCleanupTimeis set before the cleanup finishes. A failure still blocks retries for a full day.Suggested fix
- this.isRunning = true; - this.lastCleanupTime = Date.now(); + this.isRunning = true; try { @@ - return { + const result = { deletedCount: totalDeleted, userCount: userDeleted, projectCount: projectDeleted, promptsDeleted, linkedMemoriesDeleted, pinnedMemoriesSkipped: pinnedSkipped, }; + this.lastCleanupTime = Date.now(); + return result; } finally { this.isRunning = false; }Also applies to: 116-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/cleanup-service.ts` around lines 40 - 42, The code sets this.lastCleanupTime before the cleanup completes, blocking retries on failure; change the logic in the cleanup routine(s) so that this.lastCleanupTime = Date.now() is only assigned after successful completion of the cleanup (e.g., inside the successful branch of the try block or immediately after await performCleanup()), not at the start; keep this.isRunning toggles as-is but ensure isRunning is reset in a finally block; apply the same change for the second instance around lines where this.isRunning/this.lastCleanupTime appear (the other cleanup invocation).scripts/migrate-v1-to-v2.ts (4)
306-344:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDatabase handles are not reliably closed on per-DB exceptions.
If an error occurs after
new Database(dbPath), the catch path logs but does not guaranteedb.close(), risking handle leaks/locks.Suggested fix
for (const dbPath of dbs) { + let db: DatabaseType | null = null; try { - const db = new Database(dbPath); + db = new Database(dbPath); @@ - db.run("PRAGMA wal_checkpoint(TRUNCATE)"); - db.close(); + db.run("PRAGMA wal_checkpoint(TRUNCATE)"); } catch (error) { const msg = String(error); result.errors.push(`${dbPath}: ${msg}`); log("Migration: database error", { dbPath, error: msg }); + } finally { + try { + db?.close(); + } catch {} } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/migrate-v1-to-v2.ts` around lines 306 - 344, The per-database try block can throw after new Database(dbPath) and currently never guarantees db.close(); declare a Database variable (e.g., let db: Database | null = null) before the try, assign it when constructing with new Database(dbPath), and move the per-database logic (isV1Schema, addV2Columns, backfillScores, createConflictsTable, addV2Indexes, PRAGMA wal_checkpoint) inside the try; then add a finally block that checks if db is non-null and calls db.close() to ensure the handle is always closed even on exceptions; keep the existing error capture in the catch but do not rely on it for closing the connection.
354-354:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace process.env.HOME with os.homedir() for Windows compatibility.
Line 354 relies on
process.env.HOME, which is unset on Windows and falls back to".", breaking the intended home directory fallback logic. Node.js recommendsos.homedir()for cross-platform home directory resolution—it usesUSERPROFILEon Windows and$HOMEon POSIX systems.Suggested fix
+import { homedir } from "node:os"; @@ -const storagePath = process.argv[2] || join(process.env.HOME || ".", ".opencode-mem", "data"); +const storagePath = process.argv[2] || join(homedir(), ".opencode-mem", "data");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/migrate-v1-to-v2.ts` at line 354, The storagePath initialization uses process.env.HOME which is not set on Windows; replace that usage with os.homedir() (importing Node's os module if not already) so the line that defines storagePath (the const storagePath = ... statement) falls back to the real user home directory cross-platform; update the import/require at the top to include os and use os.homedir() in place of process.env.HOME.
175-176:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftFTS type mismatch: TEXT values inserted into INTEGER rowid field.
content_rowid='id'referencestranscripts.id TEXT PRIMARY KEY, but the triggers directly insert TEXT values (new.id,old.id) into FTS's rowid field, which expects INTEGER. This type inconsistency can cause FTS lookup failures and sync issues during inserts and updates.Fix: Either change
idtoINTEGER PRIMARY KEY, or refactor triggers to use SQLite's implicit rowid instead of the TEXT column directly.Affects: All FTS triggers for
transcripts_fts(insert at 198-201, delete at 190-195, update at 214-216).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/migrate-v1-to-v2.ts` around lines 175 - 176, The FTS triggers for transcripts_fts reference content_rowid='id' but the transcripts.id column is TEXT, causing TEXT values to be inserted into the FTS INTEGER rowid; fix by either making transcripts.id an INTEGER PRIMARY KEY or (preferable) change the FTS wiring to use the implicit rowid: update the CREATE VIRTUAL TABLE / content_rowid config to use the rowid (or remove content_rowid so it defaults), and modify the triggers that currently insert new.id / old.id to insert new.rowid / old.rowid instead (adjust the INSERT, DELETE and UPDATE triggers that reference new.id/old.id accordingly).
1-4:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRelative imports are incorrect for a file under
scripts/.From
scripts/migrate-v1-to-v2.ts,./src/...resolves toscripts/src/..., which does not exist. Additionally, the imports reference.jsfiles, but the actual source files are TypeScript (.ts).Change
./src/to../src/and update the extensions from.jsto.ts:Suggested fix
-import { getDatabase } from "./src/services/sqlite/sqlite-bootstrap.js"; +import { getDatabase } from "../src/services/sqlite/sqlite-bootstrap.ts"; @@ -import { log } from "./src/services/logger.js"; +import { log } from "../src/services/logger.ts";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/migrate-v1-to-v2.ts` around lines 1 - 4, The imports at the top (getDatabase, log and the node fs/path imports) use "./src/..." and ".js" extensions which resolve incorrectly from scripts/; update those module specifiers to use "../src/..." and change the TypeScript file extensions from .js to .ts (e.g., import { getDatabase } from "../src/services/sqlite/sqlite-bootstrap.ts" and import { log } from "../src/services/logger.ts") so getDatabase and log resolve to the actual TS sources while keeping the built-in node imports unchanged.src/services/ai/providers/openai-responses.ts (1)
149-149:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse resolved timeout value in the abort error message.
Line 149 references
this.config.iterationTimeout; when unset, message becomesundefinedmsdespite using fallbackiterationTimeout.Suggested fix
- error: `API request timeout (${this.config.iterationTimeout}ms)`, + error: `API request timeout (${iterationTimeout}ms)`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/ai/providers/openai-responses.ts` at line 149, The abort error message is using this.config.iterationTimeout which can be undefined; update the error text to use the resolved timeout variable used earlier in the function (e.g., iterationTimeout or resolvedIterationTimeout) so the message shows the actual millisecond value; locate the abort creation in OpenAIResponses (openai-responses.ts) and replace this.config.iterationTimeout with the local resolved timeout variable used when building the AbortController.src/services/retrieval-context.ts (1)
159-162:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard against invalid threshold values in penalty calculation.
When
threshold >= 1, Line 161 can divide by zero or produce invalid values.Suggested fix
- if (maxSimilarity > threshold) { + if (threshold >= 1) return 0; + if (maxSimilarity > threshold) { // Apply penalty proportional to how much over threshold return (maxSimilarity - threshold) / (1 - threshold); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/retrieval-context.ts` around lines 159 - 162, The penalty calculation can divide by zero when threshold >= 1; update the function in retrieval-context.ts that computes the penalty (the block using maxSimilarity and threshold) to early-return a safe value (e.g., 0) when threshold >= 1 (or clamp threshold to a valid <1 range) before performing (maxSimilarity - threshold) / (1 - threshold), ensuring you still handle normal cases where threshold < 1.src/services/tags.ts (1)
131-136:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWindows path handling in
getProjectName()fails after normalization.At Line 134,
normalize()converts forward slashes to backslashes on Windows. Splitting only on/at Line 135 leaves the full path intact instead of extracting the basename.Suggested fix
export function getProjectName(directory: string): string { - // Normalize path to handle both Unix and Windows separators. - // path.normalize on Linux does not convert backslashes, so we - // manually replace them before normalizing. - const normalized = normalize(directory.replace(/\\/g, "/")); - const parts = normalized.split("/").filter((p) => p); + const parts = directory.split(/[\\/]+/).filter((p) => p); return parts[parts.length - 1] || directory; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/tags.ts` around lines 131 - 136, getProjectName() currently normalizes the input path into the variable normalized then splits on "/" which fails on Windows because normalize() may produce backslashes; update the logic so after computing normalized you extract the basename using Node's path utilities (e.g., path.basename) or split on both separators (use path.sep or a regex like /[\\/]/) to return the last path segment; modify the code referencing normalized and parts to use this cross-platform approach so the return value is the correct project name on all OSes.src/services/ai/validators/user-profile-validator.ts (1)
30-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse presence checks instead of truthiness for optional nested section validation.
Lines 30/34/38 currently skip validation when properties contain falsy invalid values (e.g. empty string, 0, false), allowing malformed payloads to pass as valid. For example,
{ preferences: "" }bypasses validation and incorrectly returns valid, whenpreferencesmust be an array.Suggested fix
- if (data.preferences) { + if (Object.prototype.hasOwnProperty.call(data, "preferences")) { const prefErrors = this.validatePreferences(data.preferences); errors.push(...prefErrors); } - if (data.patterns) { + if (Object.prototype.hasOwnProperty.call(data, "patterns")) { const patternErrors = this.validatePatterns(data.patterns); errors.push(...patternErrors); } - if (data.workflows) { + if (Object.prototype.hasOwnProperty.call(data, "workflows")) { const workflowErrors = this.validateWorkflows(data.workflows); errors.push(...workflowErrors); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/ai/validators/user-profile-validator.ts` around lines 30 - 40, The current guards use truthiness and skip validation for falsy-but-present values (e.g. empty string), so update the conditional checks around data.preferences, data.patterns, and data.workflows to test presence explicitly (e.g. check for !== undefined && !== null or use the 'in' operator) before calling validatePreferences, validatePatterns, and validateWorkflows; this ensures payloads like { preferences: "" } still run through the validator functions and return appropriate errors.src/services/logger.ts (1)
60-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
log()non-throwing and redact sensitive fields.Line 61 calls
JSON.stringify()without error handling. Ifdatacontains circular references, this will throw and break caller execution. Additionally, raw logging of unknown objects exposes sensitive values like tokens, passwords, and API keys without redaction.Suggested fix
+function safeStringify(data: unknown): string { + try { + return JSON.stringify(data, (key, value) => { + if (/token|secret|password|api[-_]?key|authorization/i.test(key)) { + return "[REDACTED]"; + } + return value; + }); + } catch { + return '"[Unserializable data]"'; + } +} + export function log(message: string, data?: unknown) { ensureLoggerInitialized(); const logFile = getLogFilePath(); const timestamp = new Date().toISOString(); const line = data - ? `[${timestamp}] ${message}: ${JSON.stringify(data)}\n` + ? `[${timestamp}] ${message}: ${safeStringify(data)}\n` : `[${timestamp}] ${message}\n`; appendFileSync(logFile, line); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/logger.ts` around lines 60 - 63, The log() implementation currently calls JSON.stringify(data) and then appendFileSync(logFile) which can throw on circular structures and also writes raw sensitive fields; change log() to be non-throwing by wrapping serialization and file append in try/catch and swallowing/logging errors internally, and implement a safe serializer used before appendFileSync: recursively redact well-known sensitive keys (e.g., "password", "token", "apiKey", "secret", "authorization", "cookie", "accessToken") from the object returned by the serializer, then attempt JSON.stringify; if stringify still fails, fall back to a safe textual representation (e.g., util.inspect or "[Unserializable data]") so appendFileSync always receives a string and never throws to callers, keeping references to log(), appendFileSync, and logFile to locate the change.tests/vector-backends/exact-scan-backend.test.ts (1)
13-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose SQLite connections before temp directory cleanup.
Lines 43 and 78 create DB instances via
new Database(), but the afterEach hook (lines 13-18) only removes directories without closing them. Unclosed database handles can hold file locks, causing intermittent cleanup failures—particularly on Windows—sincermSynccannot remove directories with locked files.Both
BunSqliteDatabaseandBetterSqlite3Databaseimplement theclose()method (sqlite-bootstrap.ts lines 67-69, 112-114), so database cleanup is supported.Suggested fix
describe("ExactScanBackend", () => { const tempDirs: string[] = []; + const dbs: Array<{ close?: () => void }> = []; afterEach(() => { + while (dbs.length > 0) { + const db = dbs.pop(); + try { + db?.close?.(); + } catch {} + } while (tempDirs.length > 0) { const dir = tempDirs.pop(); if (dir) rmSync(dir, { recursive: true, force: true }); } }); @@ const db = new Database(join(tempDir, "test.db")); + dbs.push(db); @@ const db = new Database(join(tempDir, "test.db")); + dbs.push(db);Also applies to: 40-43, 75-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/vector-backends/exact-scan-backend.test.ts` around lines 13 - 18, The test cleanup currently only removes tempDirs and can fail when SQLite files are still open; update the tests to close any database instances before deleting directories by tracking created DB objects (those constructed with new Database() / instances of BunSqliteDatabase or BetterSqlite3Database) and calling their close() methods in afterEach prior to rmSync; specifically, add a collection (e.g., dbInstances) that test setup pushes each DB into and modify the afterEach to iterate dbInstances calling close() (await if needed) and then clear dbInstances before removing tempDirs to ensure file handles are released.src/services/secret-resolver.ts (1)
39-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParse
file://values with URL utilities instead of string slicing.Manual slicing at line 40 fails to decode URL-encoded characters and handle platform-specific paths. For example,
file:///path/my%20file.txtwill attempt to read a file literally namedmy%20file.txtinstead ofmy file.txt. UsefileURLToPath()from the Node.js standard library for correct cross-platform behavior.Suggested fix
import { existsSync, readFileSync, statSync } from "node:fs"; import { join } from "node:path"; import { homedir, platform } from "node:os"; +import { fileURLToPath } from "node:url"; @@ if (value.startsWith("file://")) { - const filePath = expandPath(value.slice(7)); + const filePath = expandPath(fileURLToPath(value));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/secret-resolver.ts` around lines 39 - 41, The code is slicing "file://" strings (value.slice(7)) which doesn't decode URL-encoded characters or handle platform file URLs; replace that logic by using Node's fileURLToPath(new URL(value)) to convert the file:// URL to a proper filesystem path, then pass that result to expandPath (instead of the sliced string). Import fileURLToPath (and URL from 'url' if needed) in secret-resolver.ts, use fileURLToPath(new URL(value)) to produce filePath, and keep the rest of the logic (reading/using filePath) unchanged.src/services/embedding.ts (1)
24-28:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftTimeout wrapper should cancel in-flight operations, not only race them.
Current timeout logic rejects the caller but leaves the underlying fetch and model work running. This can accumulate dangling requests and processing threads under load.
The remote API path (line 89) lacks an
signalparameter in fetch, and the local model path (line 108) provides no cancellation mechanism. The fix should follow the AbortController pattern already established in other providers (e.g., anthropic-messages.ts), where the signal is passed to fetch and a timer cleanup is guaranteed viafinally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/embedding.ts` around lines 24 - 28, The withTimeout function currently races the promise but doesn't cancel in-flight work; change it to create an AbortController and a timeout that calls controller.abort(), pass controller.signal into the remote fetch call (the fetch invocation in the remote API path) and into any local model request that supports cancellation (the local model invocation in the local model path), and ensure the timeout timer is cleared in a finally block so you don't leak timers; keep the exported function name withTimeout and ensure callers use the returned controller.signal for fetch and model calls, or modify withTimeout to accept a callback that receives the signal and returns the cancellable promise.src/services/sqlite/vector-search.ts (1)
560-567:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEscape LIKE wildcards in
sessionIDbefore metadata search.
sessionIDis interpolated into a LIKE pattern, so%/_can broaden matches and return unrelated records.🔒 Suggested fix
- const stmt = db.prepare(` + const stmt = db.prepare(` SELECT * FROM memories - WHERE metadata LIKE ? AND is_deprecated = 0 + WHERE metadata LIKE ? ESCAPE '\\' AND is_deprecated = 0 ORDER BY created_at DESC `); - - const rows = stmt.all(`%"sessionID":"${sessionID}"%`) as any[]; + const escapedSessionID = sessionID.replace(/[\\%_]/g, "\\$&"); + const rows = stmt.all(`%"sessionID":"${escapedSessionID}"%`) as any[];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/sqlite/vector-search.ts` around lines 560 - 567, The metadata search builds a LIKE pattern with unescaped sessionID which allows %/_ in sessionID to act as wildcards; update the code that prepares and executes the query (the db.prepare(...) call that populates stmt and the stmt.all(...) call that computes rows) to escape SQL LIKE wildcards in sessionID (replace % and _ and escape the escape char) and pass the safe pattern as a parameter (e.g., '%escapedSessionID%') and/or add an ESCAPE clause so the metadata LIKE ? only matches the literal sessionID substring.src/services/ai/opencode-provider.ts (1)
49-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace empty
catch {}blocks with explicit handling/logging.Swallowing errors in auth IO and request mutation paths can silently continue with stale credentials or malformed requests, making outages hard to diagnose.
💡 Minimal hardening example
- } catch {} + } catch (error) { + throw new Error(`Failed to read auth file '${authPath}': ${String(error)}`); + } ... - } catch {} + } catch (error) { + throw new Error(`Failed to persist refreshed OAuth token: ${String(error)}`); + }Also applies to: 115-123, 163-185, 190-202
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/ai/opencode-provider.ts` around lines 49 - 52, The empty catch blocks around file I/O and request mutation in opencode-provider.ts (e.g., the readFileSync(authPath, "utf-8") call and the other try/catch regions at lines ~115-123, ~163-185, ~190-202) must be replaced with explicit error handling: log the caught Error via the existing logger (or throw if appropriate) and add a clear context message (e.g., "failed reading auth file at authPath" or "error mutating request in <functionName>") so failures won’t be silently swallowed; locate the try/catch wrappers around readFileSync and the request mutation functions (names referenced in the diff) and ensure each catch either processLogger.error(err, ...) or rethrows after logging, and avoid leaving empty catches.src/services/ai/session/ai-session-manager.ts (1)
23-31:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftAvoid import-time crash from eager
AISessionManagerinitialization.
new AISessionManager()runs at module load, and constructor dereferencesCONFIG.storagePathimmediately. CI already shows this can throw before tests/runtime init complete.💡 Suggested fix direction (lazy singleton)
-export const aiSessionManager = new AISessionManager(); +let _aiSessionManager: AISessionManager | null = null; + +export function getAISessionManager(): AISessionManager { + if (_aiSessionManager) return _aiSessionManager; + _aiSessionManager = new AISessionManager(); + return _aiSessionManager; +}Also applies to: 225-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/ai/session/ai-session-manager.ts` around lines 23 - 31, The AISessionManager is being eagerly instantiated at module load and its constructor immediately dereferences CONFIG.storagePath (dbPath, connectionManager.getConnection, sessionRetentionMs, initDatabase), which can crash during import; change to a lazy singleton: remove the top-level new AISessionManager() and export a getAISessionManager() (or AISessionManager.getInstance()) that creates the instance on first call, and modify the constructor (or shift work into an async init method like initDatabase or initialize) so it does not access CONFIG or perform file IO during class construction but instead reads CONFIG.storagePath and opens the DB only when the singleton is first requested; update any call sites that used the exported instance (including the other occurrence referenced) to call the lazy getter and await initialization as needed.tests/tool-scope.test.ts (1)
9-39:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMock AISessionManager to prevent constructor execution during module load.
AISessionManager is exported as a singleton that instantiates immediately when imported (line 225 of
ai-session-manager.ts). Its constructor accessesCONFIG.storagePathandCONFIG.aiSessionRetentionDaysat lines 24 and 30. When the test importssrc/index.js, the import chain triggersAIProviderFactory→ai-session-manager→new AISessionManager(), which attempts to access CONFIG beforemockConfigis assigned. SincemockConfigis assigned after thevi.mock()calls (lines 41-96), the CONFIG mock receivesundefined.Add this mock before the CONFIG mock:
🔧 Proposed fix
vi.mock("../src/services/language-detector.js", () => ({ getLanguageName: () => "English" })); +vi.mock("../src/services/ai/session/ai-session-manager.js", () => ({ + aiSessionManager: { + createSession: vi.fn(), + getSession: vi.fn(), + addMessage: vi.fn(), + getMessages: vi.fn(), + getLastSequence: vi.fn(() => 0), + updateSession: vi.fn(), + cleanupExpiredSessions: vi.fn(() => 0), + }, +})); vi.mock("../src/config.js", () => ({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/tool-scope.test.ts` around lines 9 - 39, AISessionManager is being instantiated during module import and reads CONFIG before mockConfig is set; to fix, add a vi.mock for the ai-session-manager module before the CONFIG mock that replaces the real exports with a stubbed AISessionManager class (no-op constructor) and a dummy exported singleton (or named export used by imports) so the real constructor never runs during test load; reference AISessionManager, ai-session-manager (module), CONFIG, mockConfig and the import chain via AIProviderFactory/src/index.js to locate where to insert this new mock (place it above the existing CONFIG mock).tests/language-detector.test.ts (1)
9-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDuplicate test case.
This test is identical to the test on lines 5-7 (same input, same expected output). The description mentions "short Chinese (5+ chars)" but uses an 8-character string identical to the first test. Consider either removing the duplicate or testing with exactly 5 characters to verify the
minLength: 5boundary.💚 Proposed fix to test the actual boundary
it("should detect short Chinese as zh (5+ chars)", () => { - expect(detectLanguage("这是一个中文测试")).toBe("zh"); + expect(detectLanguage("这是一个中")).toBe("zh"); // exactly 5 chars });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/language-detector.test.ts` around lines 9 - 11, The test duplicates an existing case for detectLanguage using the same 8-character Chinese string; update the test in tests/language-detector.test.ts to either remove this duplicate or change the input to a true 5-character Chinese string to exercise the minLength boundary (keep the test name "should detect short Chinese as zh (5+ chars)" and assert detectLanguage(...) === "zh"); ensure the unique symbol detectLanguage is referenced and the test description matches the new input.src/services/privacy.ts (1)
1-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEdge case: nested
<private>tags may leak content.The regex
/<private>[\s\S]*?<\/private>/giuses non-greedy matching, which handles multiple separate<private>blocks correctly. However, nested tags like<private>outer<private>inner</private>outer</private>would result in[REDACTED]outer</private>— leaving content exposed.If nested tags are possible in your data, consider either:
- Documenting that nesting is unsupported, or
- Iteratively applying the regex until no matches remain.
🛡️ Iterative approach if nesting is possible
export function stripPrivateContent(content: string): string { - return content.replace(/<private>[\s\S]*?<\/private>/gi, "[REDACTED]"); + let result = content; + let prev: string; + do { + prev = result; + result = result.replace(/<private>[\s\S]*?<\/private>/gi, "[REDACTED]"); + } while (result !== prev); + return result; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/privacy.ts` around lines 1 - 3, The current stripPrivateContent function uses the regex /<private>[\s\S]*?<\/private>/gi which fails on nested <private>…</private> tags and can leak inner content; modify stripPrivateContent to repeatedly apply the regex (loop calling content.replace(...) until no replacements occur) or implement a simple stack-based parser to fully remove nested blocks, ensuring the function returns fully redacted text for inputs like "<private>outer<private>inner</private>outer</private>".src/services/ai/providers/google-gemini.ts (2)
94-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUncaught JSON.parse may throw on malformed tool call arguments.
JSON.parse(tc.function.arguments)at line 94 andJSON.parse(msg.content)at line 107 are not wrapped in try/catch. Malformed JSON from the API or stored messages will cause an unhandled exception, breaking the entire tool call flow.🐛 Proposed fix: wrap in try/catch
if (msg.toolCalls) { for (const tc of msg.toolCalls) { + try { parts.push({ functionCall: { name: tc.function.name, args: JSON.parse(tc.function.arguments), }, }); + } catch { + log("Gemini: failed to parse tool call arguments", { toolCallId: tc.id }); + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/ai/providers/google-gemini.ts` at line 94, Wrap the unguarded JSON.parse calls for tc.function.arguments and msg.content in try/catch inside the Google Gemini tool-call handling code so malformed JSON doesn't throw; on parse failure log the error via the existing logger (or processLogger), set a safe fallback (e.g., {} for args or an empty string), and propagate a controlled error or mark the tool call as failed instead of letting an exception bubble. Locate the parsing sites that reference tc.function.arguments and msg.content and replace direct JSON.parse(...) with a guarded parse function or inline try/catch that returns a safe default and records the parse error.
154-154:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAPI key in URL query string may leak in logs or error messages.
The API key is passed as a URL query parameter. While this is Google's standard approach, the URL may be logged in error scenarios (line 182-188 logs the error which could include the URL in stack traces). Consider redacting the key from any logged URLs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/ai/providers/google-gemini.ts` at line 154, The URL currently includes the API key in the query string via const url = `${baseUrl}/models/${this.config.model}:generateContent?key=${this.config.apiKey}`; to avoid leaking the key in logs, stop logging the raw url variable and instead create a redactedUrl (or build urlWithoutKey = `${baseUrl}/models/${this.config.model}:generateContent`) for any log messages; keep using the original url (or pass apiKey separately) when making the HTTP request but ensure the error logging block that references url (the catch/logging code in this file) logs redactedUrl or omits this.config.apiKey altogether; alternatively, if the API supports it, move this.config.apiKey into an Authorization header and construct the request accordingly, but in either case ensure no logs include the plaintext API key.src/services/sqlite/transcript-manager.ts (1)
74-80:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftFTS5 configuration error breaks transcript saving:
idis TEXT butcontent_rowidexpects INTEGER.The FTS5 virtual table declares
content_rowid='id', but thetranscriptstable definesidasTEXT PRIMARY KEY. This causes the triggers (lines 83-107) to fail when inserting records—they attempt to insert TEXT values into the integerrowidcolumn, resulting in "datatype mismatch" errors. This breaks thesaveTranscript()method entirely.The
searchTranscripts()method (line 193) also has a malformed JOIN onfts.rowid = t.id(integer vs. text comparison), though this function is unused.🐛 Suggested fix: use implicit rowid
// FTS5 virtual table for full-text search on messages db.run(` CREATE VIRTUAL TABLE IF NOT EXISTS transcripts_fts USING fts5( messages, - content='transcripts', - content_rowid='id' + content='transcripts' ) `);Then update triggers to use
rowidinstead ofid:CREATE TRIGGER IF NOT EXISTS transcripts_fts_insert AFTER INSERT ON transcripts BEGIN - INSERT INTO transcripts_fts(rowid, messages) - VALUES (new.id, new.messages); + INSERT INTO transcripts_fts(rowid, messages) + VALUES (new.rowid, new.messages); ENDAnd fix the search query:
- WHERE transcripts_fts MATCH ? + WHERE t.rowid IN (SELECT rowid FROM transcripts_fts WHERE transcripts_fts MATCH ?)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/sqlite/transcript-manager.ts` around lines 74 - 80, The FTS5 virtual table uses content_rowid='id' while the transcripts table defines id as TEXT, causing datatype mismatches and failing the triggers and saveTranscript(); change the FTS5 setup to use the implicit integer rowid (remove content_rowid='id' and don’t map it to the TEXT id), update the associated triggers (the INSERT/UPDATE/DELETE triggers that reference transcripts_fts and the transcripts table) to use rowid instead of id when inserting/updating the FTS table, and fix searchTranscripts() to join on fts.rowid = t.rowid (or compare integer rowid to rowid) so the JOINs and trigger operations use the implicit integer rowid consistently and avoid text/integer mismatches while leaving the external TEXT id column unchanged.src/services/memory-scoring-service.ts (1)
76-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing rollback on shard-level failures after
BEGIN TRANSACTIONWhen any error occurs before
COMMIT, the catch path logs and continues withoutROLLBACK, which can leave an open transaction and cause lock/state issues.Minimal fix
- db.run("BEGIN TRANSACTION"); + let inTxn = false; + db.run("BEGIN TRANSACTION"); + inTxn = true; ... - db.run("COMMIT"); + db.run("COMMIT"); + inTxn = false; shardsProcessed++; } catch (error) { + try { + // best-effort cleanup when transaction was started + db.run?.("ROLLBACK"); + } catch {} log("Score recalculation failed for shard", { shardId: shard.id, error: String(error), }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/memory-scoring-service.ts` around lines 76 - 163, The shard loop begins a transaction with db.run("BEGIN TRANSACTION") but on error the catch path only logs and does not call ROLLBACK, leaving transactions open; update the error handling around the shard processing (the block that uses updateStmt.run and db.run("COMMIT")) to ensure you call db.run("ROLLBACK") when an exception occurs (or use a try/catch/finally per shard) and also ensure any prepared statement (updateStmt) is finalized/closed in the finally so resources are released; specifically modify the code that contains db.run("BEGIN TRANSACTION"), updateStmt, db.run("COMMIT"), and the catch block to run db.run("ROLLBACK") on errors and finalize updateStmt in a finally.src/services/migration-service.ts (1)
251-325:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftRe-embed migration can permanently lose memories on partial failure
The source shard is deleted before reinsertion. If embedding/insertion fails for any memory, those records are dropped and the method still returns
success: true.Suggested direction
- await shardManager.deleteShard(shardInfo.shardId); - - for (const memory of tempMemories) { + let shardHadFailures = false; + for (const memory of tempMemories) { try { const vector = await embeddingService.embedWithTimeout(memory.content); ... await vectorSearch.insertVector(...); ... } catch (error) { + shardHadFailures = true; log("Migration: error re-embedding memory", { ... }); processedCount++; } } + + if (!shardHadFailures) { + await shardManager.deleteShard(shardInfo.shardId); + } else { + // keep original shard; mark migration as partial/failed + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/migration-service.ts` around lines 251 - 325, The code deletes the source shard via shardManager.deleteShard before re-inserting memories, which can permanently lose data on partial failure; change the flow in the migration routine so you first re-embed and insert all tempMemories (using embeddingService.embedWithTimeout, vectorSearch.insertVector, vectorSearch.pinMemory, shardManager.incrementVectorCount) into the target shards and only call shardManager.deleteShard(shardInfo.shardId) after all inserts succeed for that shard; additionally record per-memory failures, surface a non-success result (or include failedIds/count) in the final return instead of always returning success: true, and ensure exception handling around embed/insert updates processedCount but does not trigger source deletion for partially failed shards.src/services/auto-capture.ts (1)
30-75:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winReset claimed prompts on early exit to prevent permanent deadlock
After
claimPrompt()succeeds (setscaptured = 2), the code has multiple early-exit paths (lines 31, 43, 50, 56, 62) and an error throw (line 35) that leave the prompt in a claimed state. SincegetLastUncapturedPrompt()only retrieves prompts withcaptured = 0, these claimed prompts become permanently stuck and unprocessable.Add a finally block to reset the claim if the prompt was never successfully marked as captured or deleted:
Suggested fix
let claimedPromptId: string | null = null; try { // ... existing code ... claimedPromptId = prompt.id; if (!userPromptManager.claimPrompt(prompt.id)) { claimedPromptId = null; return; } // ... rest of flow ... } finally { isCaptureRunning = false; if (claimedPromptId) { // Reset claim if capture didn't complete userPromptManager.resetPromptClaim(claimedPromptId); } }This requires implementing
resetPromptClaim()to setcaptured = 0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/auto-capture.ts` around lines 30 - 75, The prompt claiming flow in auto-capture leaves prompts permanently stuck because claimPrompt(prompt.id) sets captured=2 but several early returns and the thrown error never reset that state; modify the capture routine (the async function containing the call to userPromptManager.claimPrompt, the session fetch via ctx.client.session.messages, extractAIContent, buildMarkdownContext, and generateSummary) to track a claimedPromptId variable when claimPrompt succeeds, wrap the existing logic in try/finally, and in the finally block set isCaptureRunning = false and call a new userPromptManager.resetPromptClaim(claimedPromptId) if claimedPromptId is set and the prompt was not successfully deleted or marked captured; also implement userPromptManager.resetPromptClaim(promptId) to set captured = 0 so claimed prompts are released on all early-exit paths.src/services/memory-scoring.ts (1)
205-210:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
gflag from shared regexes used with.test()to prevent stateful scoring bugs.Lines 205-210 and 214-215 define global regexes that are reused in loops at lines 455 and 464 via
.test(). With thegflag,lastIndexis mutated across calls, causinghasNegationandhasActionto return nondeterministic results.Suggested fix
const NEGATION_PATTERNS = [ - /\b(not|no|never|none|nothing|nobody|nowhere|neither|nor)\b/gi, - /\b(don't|doesn't|didn't|won't|wouldn't|shouldn't|couldn't|can't|cannot)\b/gi, - /\b(removed|deleted|reverted|undone|cancelled|canceled|disabled|turned off)\b/gi, - /\b(un|dis|mis|non)[a-z]+\b/gi, - /\b(false|incorrect|wrong|invalid|failed|error)\b/gi, + /\b(not|no|never|none|nothing|nobody|nowhere|neither|nor)\b/i, + /\b(don't|doesn't|didn't|won't|wouldn't|shouldn't|couldn't|can't|cannot)\b/i, + /\b(removed|deleted|reverted|undone|cancelled|canceled|disabled|turned off)\b/i, + /\b(un|dis|mis|non)[a-z]+\b/i, + /\b(false|incorrect|wrong|invalid|failed|error)\b/i, ]; const ACTION_PATTERNS = [ - /\b(added|created|implemented|built|developed|wrote|wrote|configured|enabled|fixed|resolved|solved)\b/gi, - /\b(true|correct|valid|success|working|active|enabled|on)\b/gi, + /\b(added|created|implemented|built|developed|wrote|wrote|configured|enabled|fixed|resolved|solved)\b/i, + /\b(true|correct|valid|success|working|active|enabled|on)\b/i, ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/memory-scoring.ts` around lines 205 - 210, The shared regexes (the arrays of patterns used for negation/action detection) are declared with the global "g" flag and are later reused with RegExp.test() in hasNegation and hasAction, causing stateful failures due to lastIndex; remove the "g" flag from those regex literals (or alternatively ensure each test uses a new RegExp instance) so that hasNegation and hasAction produce deterministic results when calling .test() repeatedly on the same patterns.
ZeR020
changed the title
ZeR020
changed the title
Version bump, changelog, and docs update for release. Added: - file://~/ path expansion in *ApiKey config fields (#34) - initConfig() empty-config guard preserving CONFIG on transient I/O failure (#35) Fixed: - initConfig() silent CONFIG reset to defaults (#35, #36) - file://~/ throws on Linux — ~ expanded before URL parsing (#34) - Vitest critical CVE GHSA-5xrq-8626-4rwp (dev-only, updated to 3.2.6) Removed (dead code, zero consumers): - NSWBackend + nsw/nsw-first factory branches (production-unreachable) - supportsSession() abstract method (always returned true) - profile-utils.ts (zero importers) - aiSessionManager backward-compatible Proxy export - getSupportedProviders(), startCleanupSchedule/stopCleanupSchedule - ConflictCheckLock class (inlined as Set) Changed: - iso-639-3 full dataset → Intl.DisplayNames - Dynamic imports → static in auto-capture.ts and handlers/memory.ts - .gitignore reorganized (280 → 73 lines) - localStorage API key storage documented (code scanning #7) Closed: #34, #35, #36, #37
1 participant
Audit-Only PR — DO NOT MERGE
This PR was created to trigger a comprehensive CodeRabbit AI review of the entire opencode-mem0 codebase.
Scope (83 files)
src/tests/scripts/Review Focus
anycasts, missing null checks, unsafe assertionsRuntime Context
bun:sqlite||better-sqlite3Bun.serve()||node:http@coderabbitai Please perform a full audit of all changes in this PR.
Summary by CodeRabbit