Skip to content

Commit 99be0ca

Browse files
committed
Address second round of PR #169 review comments
- Bound metadataCache to 256 entries with oldest-first eviction - Make SqliteStore.initialize() idempotent (close existing db first) - Fix TOCTOU in initialize(): try readFileSync directly instead of existsSync - Atomic flush: write to temp file + renameSync to prevent corruption - Clarify annotation_search uses substring LIKE matching, not FTS - Close store in monitoring-tools test afterEach to prevent timer leaks
1 parent 0d4908c commit 99be0ca

File tree

6 files changed

+88
-59
lines changed

6 files changed

+88
-59
lines changed

server/dist/codeql-development-mcp-server.js

Lines changed: 52 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -184851,6 +184851,7 @@ var BUILT_IN_EVALUATORS = {
184851184851
"csv-decode": "CSV format decoder for query results",
184852184852
"mermaid-graph": "Mermaid diagram generator for @kind graph queries (like PrintAST)"
184853184853
};
184854+
var METADATA_CACHE_MAX = 256;
184854184855
var metadataCache = /* @__PURE__ */ new Map();
184855184856
async function extractQueryMetadata(queryPath) {
184856184857
try {
@@ -184874,6 +184875,10 @@ async function extractQueryMetadata(queryPath) {
184874184875
if (tagsMatch) {
184875184876
metadata.tags = tagsMatch[1].split(/\s+/).filter((t) => t.length > 0);
184876184877
}
184878+
if (metadataCache.size >= METADATA_CACHE_MAX) {
184879+
const firstKey = metadataCache.keys().next().value;
184880+
if (firstKey !== void 0) metadataCache.delete(firstKey);
184881+
}
184877184882
metadataCache.set(queryPath, { mtime, metadata });
184878184883
return metadata;
184879184884
} catch (error2) {
@@ -185117,7 +185122,7 @@ import { randomUUID as randomUUID2 } from "crypto";
185117185122
// src/lib/sqlite-store.ts
185118185123
var import_sql_asm = __toESM(require_sql_asm(), 1);
185119185124
init_logger();
185120-
import { existsSync as existsSync6, mkdirSync as mkdirSync5, readFileSync as readFileSync5, writeFileSync as writeFileSync2 } from "fs";
185125+
import { mkdirSync as mkdirSync5, readFileSync as readFileSync5, renameSync, writeFileSync as writeFileSync2 } from "fs";
185121185126
import { join as join8 } from "path";
185122185127
var SqliteStore = class _SqliteStore {
185123185128
db = null;
@@ -185133,19 +185138,18 @@ var SqliteStore = class _SqliteStore {
185133185138
}
185134185139
/**
185135185140
* Initialize the database. Must be called (and awaited) before any other method.
185141+
* Safe to call more than once — an already-open database is closed first.
185136185142
*/
185137185143
async initialize() {
185144+
if (this.db) {
185145+
this.close();
185146+
}
185138185147
mkdirSync5(this.storageDir, { recursive: true });
185139185148
const SQL = await (0, import_sql_asm.default)();
185140-
if (existsSync6(this.dbPath)) {
185141-
try {
185142-
const fileBuffer = readFileSync5(this.dbPath);
185143-
this.db = new SQL.Database(fileBuffer);
185144-
} catch {
185145-
logger.warn("Failed to read existing database, creating new one");
185146-
this.db = new SQL.Database();
185147-
}
185148-
} else {
185149+
try {
185150+
const fileBuffer = readFileSync5(this.dbPath);
185151+
this.db = new SQL.Database(fileBuffer);
185152+
} catch {
185149185153
this.db = new SQL.Database();
185150185154
}
185151185155
this.createSchema();
@@ -185244,6 +185248,9 @@ var SqliteStore = class _SqliteStore {
185244185248
}
185245185249
/**
185246185250
* Write the in-memory database to disk.
185251+
*
185252+
* Uses write-to-temp + atomic rename so a crash mid-write cannot
185253+
* corrupt the database file.
185247185254
*/
185248185255
flush() {
185249185256
if (this.flushTimer) {
@@ -185253,7 +185260,9 @@ var SqliteStore = class _SqliteStore {
185253185260
if (!this.db) return;
185254185261
const data = this.db.export();
185255185262
const buffer = Buffer.from(data);
185256-
writeFileSync2(this.dbPath, buffer);
185263+
const tmpPath = this.dbPath + ".tmp";
185264+
writeFileSync2(tmpPath, buffer);
185265+
renameSync(tmpPath, this.dbPath);
185257185266
this.dirty = false;
185258185267
}
185259185268
/**
@@ -186379,7 +186388,7 @@ Warning: Query processing error - ${error2}`
186379186388
// src/lib/cli-tool-registry.ts
186380186389
init_package_paths();
186381186390
init_temp_dir();
186382-
import { writeFileSync as writeFileSync4, rmSync, existsSync as existsSync7, mkdirSync as mkdirSync8 } from "fs";
186391+
import { writeFileSync as writeFileSync4, rmSync, existsSync as existsSync6, mkdirSync as mkdirSync8 } from "fs";
186383186392
import { dirname as dirname5, isAbsolute as isAbsolute4, join as join10, resolve as resolve4 } from "path";
186384186393
var defaultCLIResultProcessor = (result, _params) => {
186385186394
if (!result.success) {
@@ -186706,7 +186715,7 @@ function registerCLITool(server, definition) {
186706186715
cwd = isAbsolute4(rawCwd) ? rawCwd : resolve4(getUserWorkspaceDir(), rawCwd);
186707186716
}
186708186717
const defaultExamplesPath = resolve4(packageRootDir, "ql", "javascript", "examples");
186709-
const additionalPacksPath = process.env.CODEQL_ADDITIONAL_PACKS || (existsSync7(defaultExamplesPath) ? defaultExamplesPath : void 0);
186718+
const additionalPacksPath = process.env.CODEQL_ADDITIONAL_PACKS || (existsSync6(defaultExamplesPath) ? defaultExamplesPath : void 0);
186710186719
if (additionalPacksPath && (name === "codeql_test_run" || name === "codeql_query_run" || name === "codeql_query_compile" || name === "codeql_database_analyze")) {
186711186720
options["additional-packs"] = additionalPacksPath;
186712186721
}
@@ -186727,7 +186736,7 @@ function registerCLITool(server, definition) {
186727186736
}
186728186737
if ((name === "codeql_query_run" || name === "codeql_database_analyze") && result.success && queryLogDir) {
186729186738
const evalLogPath = options["evaluator-log"];
186730-
if (evalLogPath && existsSync7(evalLogPath)) {
186739+
if (evalLogPath && existsSync6(evalLogPath)) {
186731186740
try {
186732186741
const summaryPath = evalLogPath.replace(/\.jsonl$/, ".summary.jsonl");
186733186742
const summaryResult = await executeCodeQLCommand(
@@ -190033,7 +190042,7 @@ var codeqlGenerateQueryHelpTool = {
190033190042
};
190034190043

190035190044
// src/tools/codeql/list-databases.ts
190036-
import { existsSync as existsSync9, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync4 } from "fs";
190045+
import { existsSync as existsSync8, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync4 } from "fs";
190037190046
import { join as join12 } from "path";
190038190047

190039190048
// src/lib/discovery-config.ts
@@ -190086,7 +190095,7 @@ function parseDatabaseYml(ymlPath) {
190086190095
async function discoverDatabases(baseDirs, language) {
190087190096
const databases = [];
190088190097
for (const baseDir of baseDirs) {
190089-
if (!existsSync9(baseDir)) {
190098+
if (!existsSync8(baseDir)) {
190090190099
continue;
190091190100
}
190092190101
let entries;
@@ -190105,7 +190114,7 @@ async function discoverDatabases(baseDirs, language) {
190105190114
continue;
190106190115
}
190107190116
const ymlPath = join12(entryPath, "codeql-database.yml");
190108-
if (!existsSync9(ymlPath)) {
190117+
if (!existsSync8(ymlPath)) {
190109190118
continue;
190110190119
}
190111190120
const metadata = parseDatabaseYml(ymlPath);
@@ -190187,15 +190196,15 @@ function registerListDatabasesTool(server) {
190187190196
}
190188190197

190189190198
// src/tools/codeql/list-mrva-run-results.ts
190190-
import { existsSync as existsSync10, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync5 } from "fs";
190199+
import { existsSync as existsSync9, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync5 } from "fs";
190191190200
import { join as join13 } from "path";
190192190201
init_logger();
190193190202
var NUMERIC_DIR_PATTERN = /^\d+$/;
190194190203
var SKIP_DIRS = /* @__PURE__ */ new Set([".DS_Store", "exported-results"]);
190195190204
async function discoverMrvaRunResults(resultsDirs, runId) {
190196190205
const results = [];
190197190206
for (const dir of resultsDirs) {
190198-
if (!existsSync10(dir)) {
190207+
if (!existsSync9(dir)) {
190199190208
continue;
190200190209
}
190201190210
let entries;
@@ -190221,7 +190230,7 @@ async function discoverMrvaRunResults(resultsDirs, runId) {
190221190230
}
190222190231
let timestamp2;
190223190232
const timestampPath = join13(entryPath, "timestamp");
190224-
if (existsSync10(timestampPath)) {
190233+
if (existsSync9(timestampPath)) {
190225190234
try {
190226190235
timestamp2 = readFileSync9(timestampPath, "utf-8").trim();
190227190236
} catch {
@@ -190277,7 +190286,7 @@ function discoverRepoResults(runPath) {
190277190286
let analysisStatus;
190278190287
let resultCount;
190279190288
const repoTaskPath = join13(repoPath, "repo_task.json");
190280-
if (existsSync10(repoTaskPath)) {
190289+
if (existsSync9(repoTaskPath)) {
190281190290
try {
190282190291
const raw = readFileSync9(repoTaskPath, "utf-8");
190283190292
const task = JSON.parse(raw);
@@ -190290,8 +190299,8 @@ function discoverRepoResults(runPath) {
190290190299
} catch {
190291190300
}
190292190301
}
190293-
const hasSarif = existsSync10(join13(repoPath, "results", "results.sarif"));
190294-
const hasBqrs = existsSync10(join13(repoPath, "results", "results.bqrs"));
190302+
const hasSarif = existsSync9(join13(repoPath, "results", "results.sarif"));
190303+
const hasBqrs = existsSync9(join13(repoPath, "results", "results.bqrs"));
190295190304
repos.push({
190296190305
analysisStatus,
190297190306
fullName,
@@ -190374,7 +190383,7 @@ function registerListMrvaRunResultsTool(server) {
190374190383
}
190375190384

190376190385
// src/tools/codeql/list-query-run-results.ts
190377-
import { existsSync as existsSync11, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync6 } from "fs";
190386+
import { existsSync as existsSync10, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync6 } from "fs";
190378190387
import { join as join14 } from "path";
190379190388
init_logger();
190380190389
var QUERY_RUN_DIR_PATTERN = /^(.+\.ql)-(.+)$/;
@@ -190411,7 +190420,7 @@ async function discoverQueryRunResults(resultsDirs, filter) {
190411190420
const normalizedFilter = typeof filter === "string" ? { queryName: filter } : filter;
190412190421
const results = [];
190413190422
for (const dir of resultsDirs) {
190414-
if (!existsSync11(dir)) {
190423+
if (!existsSync10(dir)) {
190415190424
continue;
190416190425
}
190417190426
let entries;
@@ -190437,14 +190446,14 @@ async function discoverQueryRunResults(resultsDirs, filter) {
190437190446
if (normalizedFilter?.queryName && name !== normalizedFilter.queryName) {
190438190447
continue;
190439190448
}
190440-
const hasEvaluatorLog = existsSync11(join14(entryPath, "evaluator-log.jsonl"));
190441-
const hasBqrs = existsSync11(join14(entryPath, "results.bqrs"));
190442-
const hasSarif = existsSync11(join14(entryPath, "results-interpreted.sarif"));
190443-
const hasQueryLog = existsSync11(join14(entryPath, "query.log"));
190444-
const hasSummaryLog = existsSync11(join14(entryPath, "evaluator-log.summary.jsonl"));
190449+
const hasEvaluatorLog = existsSync10(join14(entryPath, "evaluator-log.jsonl"));
190450+
const hasBqrs = existsSync10(join14(entryPath, "results.bqrs"));
190451+
const hasSarif = existsSync10(join14(entryPath, "results-interpreted.sarif"));
190452+
const hasQueryLog = existsSync10(join14(entryPath, "query.log"));
190453+
const hasSummaryLog = existsSync10(join14(entryPath, "evaluator-log.summary.jsonl"));
190445190454
let timestamp2;
190446190455
const timestampPath = join14(entryPath, "timestamp");
190447-
if (existsSync11(timestampPath)) {
190456+
if (existsSync10(timestampPath)) {
190448190457
try {
190449190458
timestamp2 = readFileSync10(timestampPath, "utf-8").trim();
190450190459
} catch {
@@ -190626,7 +190635,7 @@ var codeqlPackLsTool = {
190626190635
};
190627190636

190628190637
// src/tools/codeql/profile-codeql-query-from-logs.ts
190629-
import { existsSync as existsSync12, mkdirSync as mkdirSync9, writeFileSync as writeFileSync5 } from "fs";
190638+
import { existsSync as existsSync11, mkdirSync as mkdirSync9, writeFileSync as writeFileSync5 } from "fs";
190630190639
import { basename as basename6, dirname as dirname7, join as join15 } from "path";
190631190640

190632190641
// src/lib/evaluator-log-parser.ts
@@ -191063,7 +191072,7 @@ function registerProfileCodeQLQueryFromLogsTool(server) {
191063191072
try {
191064191073
const { evaluatorLog, outputDir, topN } = params;
191065191074
const effectiveTopN = topN ?? 20;
191066-
if (!existsSync12(evaluatorLog)) {
191075+
if (!existsSync11(evaluatorLog)) {
191067191076
return {
191068191077
content: [
191069191078
{
@@ -191116,7 +191125,7 @@ function registerProfileCodeQLQueryFromLogsTool(server) {
191116191125
// src/tools/codeql/profile-codeql-query.ts
191117191126
init_cli_executor();
191118191127
init_logger();
191119-
import { writeFileSync as writeFileSync6, existsSync as existsSync13 } from "fs";
191128+
import { writeFileSync as writeFileSync6, existsSync as existsSync12 } from "fs";
191120191129
import { createReadStream as createReadStream2 } from "fs";
191121191130
import { join as join16, dirname as dirname8, basename as basename7 } from "path";
191122191131
import { mkdirSync as mkdirSync10 } from "fs";
@@ -191319,7 +191328,7 @@ function registerProfileCodeQLQueryTool(server) {
191319191328
isError: true
191320191329
};
191321191330
}
191322-
if (existsSync13(bqrsPath)) {
191331+
if (existsSync12(bqrsPath)) {
191323191332
try {
191324191333
const sarifResult = await executeCodeQLCommand(
191325191334
"bqrs interpret",
@@ -191334,7 +191343,7 @@ function registerProfileCodeQLQueryTool(server) {
191334191343
}
191335191344
}
191336191345
}
191337-
if (!existsSync13(logPath)) {
191346+
if (!existsSync12(logPath)) {
191338191347
return {
191339191348
content: [
191340191349
{
@@ -191365,7 +191374,7 @@ function registerProfileCodeQLQueryTool(server) {
191365191374
if (bqrsPath) {
191366191375
outputFiles.push(`Query Results (BQRS): ${bqrsPath}`);
191367191376
}
191368-
if (sarifPath && existsSync13(sarifPath)) {
191377+
if (sarifPath && existsSync12(sarifPath)) {
191369191378
outputFiles.push(`Query Results (SARIF): ${sarifPath}`);
191370191379
}
191371191380
const responseText = [
@@ -191563,7 +191572,7 @@ function registerQuickEvaluateTool(server) {
191563191572

191564191573
// src/tools/codeql/read-database-source.ts
191565191574
var import_adm_zip = __toESM(require_adm_zip(), 1);
191566-
import { existsSync as existsSync14, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync7 } from "fs";
191575+
import { existsSync as existsSync13, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync7 } from "fs";
191567191576
import { join as join18, resolve as resolve7 } from "path";
191568191577
import { fileURLToPath as fileURLToPath2 } from "url";
191569191578
init_logger();
@@ -191645,13 +191654,13 @@ function applyLineRange(content, startLine, endLine) {
191645191654
async function readDatabaseSource(params) {
191646191655
const { databasePath, endLine, filePath, maxEntries, prefix, startLine } = params;
191647191656
const resolvedDbPath = resolve7(databasePath);
191648-
if (!existsSync14(resolvedDbPath)) {
191657+
if (!existsSync13(resolvedDbPath)) {
191649191658
throw new Error(`Database path does not exist: ${databasePath}`);
191650191659
}
191651191660
const srcZipPath = join18(resolvedDbPath, "src.zip");
191652191661
const srcDirPath = join18(resolvedDbPath, "src");
191653-
const hasSrcZip = existsSync14(srcZipPath);
191654-
const hasSrcDir = existsSync14(srcDirPath);
191662+
const hasSrcZip = existsSync13(srcZipPath);
191663+
const hasSrcDir = existsSync13(srcDirPath);
191655191664
if (!hasSrcZip && !hasSrcDir) {
191656191665
throw new Error(
191657191666
`No source archive found in database: expected src.zip or src/ in ${databasePath}`
@@ -195334,9 +195343,9 @@ function registerAnnotationDeleteTool(server) {
195334195343
function registerAnnotationSearchTool(server) {
195335195344
server.tool(
195336195345
"annotation_search",
195337-
"Full-text search across annotation content, metadata, and labels.",
195346+
"Substring search across annotation content, metadata, and labels (case-insensitive SQL LIKE matching).",
195338195347
{
195339-
query: external_exports.string().describe("Search term (matched against content, metadata, and label)."),
195348+
query: external_exports.string().describe("Search term \u2014 matched as a substring against content, metadata, and label."),
195340195349
category: external_exports.string().optional().describe("Restrict search to a specific category."),
195341195350
limit: external_exports.number().optional().describe("Maximum number of results (default: 50).")
195342195351
},

server/dist/codeql-development-mcp-server.js.map

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/src/lib/query-results-evaluator.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ export type BuiltInEvaluator = keyof typeof BUILT_IN_EVALUATORS;
3737
/**
3838
* In-memory cache for extracted query metadata, keyed by file path.
3939
* Stores the file modification time to invalidate when the file changes.
40+
* Bounded to {@link METADATA_CACHE_MAX} entries; oldest entries are evicted
41+
* when the limit is reached (Map iteration order = insertion order).
4042
*/
43+
const METADATA_CACHE_MAX = 256;
4144
const metadataCache = new Map<string, { mtime: number; metadata: QueryMetadata }>();
4245

4346
/**
@@ -75,6 +78,11 @@ export async function extractQueryMetadata(queryPath: string): Promise<QueryMeta
7578
metadata.tags = tagsMatch[1].split(/\s+/).filter(t => t.length > 0);
7679
}
7780

81+
// Evict oldest entries when the cache exceeds the size limit.
82+
if (metadataCache.size >= METADATA_CACHE_MAX) {
83+
const firstKey = metadataCache.keys().next().value;
84+
if (firstKey !== undefined) metadataCache.delete(firstKey);
85+
}
7886
metadataCache.set(queryPath, { mtime, metadata });
7987
return metadata;
8088
} catch (error) {

server/src/lib/sqlite-store.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
import initSqlJs from 'sql.js/dist/sql-asm.js';
1515
import type { Database as SqlJsDatabase } from 'sql.js';
16-
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs';
16+
import { mkdirSync, readFileSync, renameSync, writeFileSync } from 'fs';
1717
import { join } from 'path';
1818
import { logger } from '../utils/logger';
1919

@@ -65,21 +65,22 @@ export class SqliteStore {
6565

6666
/**
6767
* Initialize the database. Must be called (and awaited) before any other method.
68+
* Safe to call more than once — an already-open database is closed first.
6869
*/
6970
async initialize(): Promise<void> {
71+
if (this.db) {
72+
this.close();
73+
}
74+
7075
mkdirSync(this.storageDir, { recursive: true });
7176

7277
const SQL = await initSqlJs();
7378

74-
if (existsSync(this.dbPath)) {
75-
try {
76-
const fileBuffer = readFileSync(this.dbPath);
77-
this.db = new SQL.Database(fileBuffer);
78-
} catch {
79-
logger.warn('Failed to read existing database, creating new one');
80-
this.db = new SQL.Database();
81-
}
82-
} else {
79+
try {
80+
const fileBuffer = readFileSync(this.dbPath);
81+
this.db = new SQL.Database(fileBuffer);
82+
} catch {
83+
// File doesn't exist or is unreadable — start with a fresh database.
8384
this.db = new SQL.Database();
8485
}
8586

@@ -194,6 +195,9 @@ export class SqliteStore {
194195

195196
/**
196197
* Write the in-memory database to disk.
198+
*
199+
* Uses write-to-temp + atomic rename so a crash mid-write cannot
200+
* corrupt the database file.
197201
*/
198202
flush(): void {
199203
if (this.flushTimer) {
@@ -203,7 +207,9 @@ export class SqliteStore {
203207
if (!this.db) return;
204208
const data = this.db.export();
205209
const buffer = Buffer.from(data);
206-
writeFileSync(this.dbPath, buffer);
210+
const tmpPath = this.dbPath + '.tmp';
211+
writeFileSync(tmpPath, buffer);
212+
renameSync(tmpPath, this.dbPath);
207213
this.dirty = false;
208214
}
209215

0 commit comments

Comments
 (0)