Skip to content

Commit e8ef061

Browse files
data-douserdata-douser
committed
Address PR #169 review #4024692955
- annotation_list/annotation_search limit: z.number().int().positive() - annotation_list offset: z.number().int().nonnegative() - audit_list_findings limit: z.number().int().positive().max(1000) - query_results_cache_lookup: add limit param (default 50, max 500) - listAnnotations: wrap FTS MATCH in try/catch; return [] on syntax error - listAnnotations: emit LIMIT -1 when only offset is provided (SQLite requirement) - listCacheEntries: add limit filter support - SessionDataManager.initialize(): recreate SqliteStore when storageLocation changed since construction (fixes test isolation and runtime config updates) - sqlite-store tests: 3 new cases for FTS error, offset-only, cache limit
1 parent c47757a commit e8ef061

File tree

7 files changed

+108
-27
lines changed

7 files changed

+108
-27
lines changed

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

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -183597,17 +183597,29 @@ var SqliteStore = class _SqliteStore {
183597183597
params.$limit = filter.limit;
183598183598
}
183599183599
if (filter?.offset !== void 0) {
183600+
if (filter?.limit === void 0) {
183601+
sql += " LIMIT -1";
183602+
}
183600183603
sql += " OFFSET $offset";
183601183604
params.$offset = filter.offset;
183602183605
}
183603183606
const stmt = db.prepare(sql);
183604-
stmt.bind(params);
183605-
const results = [];
183606-
while (stmt.step()) {
183607-
results.push(stmt.getAsObject());
183607+
try {
183608+
stmt.bind(params);
183609+
const results = [];
183610+
while (stmt.step()) {
183611+
results.push(stmt.getAsObject());
183612+
}
183613+
return results;
183614+
} catch (err) {
183615+
if (filter?.search) {
183616+
logger.warn("FTS MATCH query failed (invalid syntax?); returning empty results:", err);
183617+
return [];
183618+
}
183619+
throw err;
183620+
} finally {
183621+
stmt.free();
183608183622
}
183609-
stmt.free();
183610-
return results;
183611183623
}
183612183624
/**
183613183625
* Update an annotation's content, label, and/or metadata.
@@ -183866,6 +183878,10 @@ var SqliteStore = class _SqliteStore {
183866183878
sql += " WHERE " + conditions.join(" AND ");
183867183879
}
183868183880
sql += " ORDER BY created_at DESC";
183881+
if (filter?.limit !== void 0) {
183882+
sql += " LIMIT $limit";
183883+
params.$limit = filter.limit;
183884+
}
183869183885
const stmt = db.prepare(sql);
183870183886
stmt.bind(params);
183871183887
const results = [];
@@ -184073,6 +184089,12 @@ var SessionDataManager = class {
184073184089
*/
184074184090
async initialize() {
184075184091
try {
184092+
const storageDir = this.getConfig().storageLocation;
184093+
if (storageDir !== this.storageDir) {
184094+
this.store.close();
184095+
this.storageDir = storageDir;
184096+
this.store = new SqliteStore(this.storageDir);
184097+
}
184076184098
await this.store.initialize();
184077184099
const count = this.store.countSessions();
184078184100
logger.info(`Session data manager initialized with ${count} sessions`);
@@ -193455,8 +193477,8 @@ function registerAnnotationListTool(server) {
193455193477
category: external_exports.string().optional().describe("Filter by annotation category."),
193456193478
entityKey: external_exports.string().optional().describe("Filter by exact entity key."),
193457193479
entityKeyPrefix: external_exports.string().optional().describe('Filter by entity key prefix (e.g. "repo:owner/name").'),
193458-
limit: external_exports.number().optional().describe("Maximum number of results (default: 100)."),
193459-
offset: external_exports.number().optional().describe("Number of results to skip.")
193480+
limit: external_exports.number().int().positive().optional().describe("Maximum number of results (default: 100)."),
193481+
offset: external_exports.number().int().nonnegative().optional().describe("Number of results to skip.")
193460193482
},
193461193483
async ({ category, entityKey, entityKeyPrefix, limit, offset }) => {
193462193484
const store = sessionDataManager.getStore();
@@ -193518,7 +193540,7 @@ function registerAnnotationSearchTool(server) {
193518193540
{
193519193541
search: external_exports.string().describe("Full-text search query matched against annotation content, metadata, and label (SQLite FTS MATCH syntax; use * for prefix matching)."),
193520193542
category: external_exports.string().optional().describe("Restrict search to a specific category."),
193521-
limit: external_exports.number().optional().describe("Maximum number of results (default: 50).")
193543+
limit: external_exports.number().int().positive().optional().describe("Maximum number of results (default: 50).")
193522193544
},
193523193545
async ({ search, category, limit }) => {
193524193546
const store = sessionDataManager.getStore();
@@ -193603,7 +193625,7 @@ function registerAuditListFindingsTool(server) {
193603193625
{
193604193626
owner: external_exports.string().describe("Repository owner."),
193605193627
repo: external_exports.string().describe("Repository name."),
193606-
limit: external_exports.number().optional().describe("Maximum number of results.")
193628+
limit: external_exports.number().int().positive().max(1e3).optional().describe("Maximum number of results (1\u20131000).")
193607193629
},
193608193630
async ({ owner, repo, limit }) => {
193609193631
const store = sessionDataManager.getStore();
@@ -193712,9 +193734,10 @@ function registerQueryResultsCacheLookupTool(server) {
193712193734
cacheKey: external_exports.string().optional().describe("Look up by exact cache key (if known)."),
193713193735
queryName: external_exports.string().optional().describe('Query name to search for (e.g. "PrintAST", "CallGraphFrom").'),
193714193736
databasePath: external_exports.string().optional().describe("Database path to search for."),
193715-
language: external_exports.string().optional().describe('Filter by language (e.g. "cpp", "javascript").')
193737+
language: external_exports.string().optional().describe('Filter by language (e.g. "cpp", "javascript").'),
193738+
limit: external_exports.number().int().positive().max(500).optional().describe("Maximum number of cache entries to return when listing by filter (default: 50, max: 500).")
193716193739
},
193717-
async ({ cacheKey: cacheKey2, queryName, databasePath, language }) => {
193740+
async ({ cacheKey: cacheKey2, queryName, databasePath, language, limit }) => {
193718193741
const store = sessionDataManager.getStore();
193719193742
if (cacheKey2) {
193720193743
const meta = store.getCacheEntryMeta(cacheKey2);
@@ -193723,7 +193746,7 @@ function registerQueryResultsCacheLookupTool(server) {
193723193746
}
193724193747
return { content: [{ type: "text", text: JSON.stringify({ cached: false, cacheKey: cacheKey2 }) }] };
193725193748
}
193726-
const entries = store.listCacheEntries({ queryName, databasePath, language });
193749+
const entries = store.listCacheEntries({ queryName, databasePath, language, limit: limit ?? 50 });
193727193750
if (entries.length === 0) {
193728193751
return { content: [{ type: "text", text: JSON.stringify({ cached: false, queryName, databasePath, language }) }] };
193729193752
}

‎server/src/lib/session-data-manager.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ export class SessionDataManager {
4949
*/
5050
async initialize(): Promise<void> {
5151
try {
52+
// (Re)create the store if storageLocation changed since construction
53+
// (e.g. via updateConfig or test mocks), closing the previous store first.
54+
const storageDir = this.getConfig().storageLocation;
55+
if (storageDir !== this.storageDir) {
56+
this.store.close();
57+
this.storageDir = storageDir;
58+
this.store = new SqliteStore(this.storageDir);
59+
}
5260
await this.store.initialize();
5361
const count = this.store.countSessions();
5462
logger.info(`Session data manager initialized with ${count} sessions`);

‎server/src/lib/sqlite-store.ts‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -472,19 +472,33 @@ export class SqliteStore {
472472
params.$limit = filter.limit;
473473
}
474474
if (filter?.offset !== undefined) {
475+
if (filter?.limit === undefined) {
476+
// SQLite requires LIMIT when OFFSET is used; -1 means unlimited.
477+
sql += ' LIMIT -1';
478+
}
475479
sql += ' OFFSET $offset';
476480
params.$offset = filter.offset;
477481
}
478482

479483
const stmt = db.prepare(sql);
480-
stmt.bind(params);
481-
482-
const results: Annotation[] = [];
483-
while (stmt.step()) {
484-
results.push(stmt.getAsObject() as unknown as Annotation);
484+
try {
485+
stmt.bind(params);
486+
const results: Annotation[] = [];
487+
while (stmt.step()) {
488+
results.push(stmt.getAsObject() as unknown as Annotation);
489+
}
490+
return results;
491+
} catch (err) {
492+
if (filter?.search) {
493+
// Invalid FTS MATCH syntax (e.g. trailing operator) — return an empty
494+
// result set rather than propagating the parse error to callers.
495+
logger.warn('FTS MATCH query failed (invalid syntax?); returning empty results:', err);
496+
return [];
497+
}
498+
throw err;
499+
} finally {
500+
stmt.free();
485501
}
486-
stmt.free();
487-
return results;
488502
}
489503

490504
/**
@@ -793,6 +807,7 @@ export class SqliteStore {
793807
queryName?: string;
794808
databasePath?: string;
795809
language?: string;
810+
limit?: number;
796811
}): Array<{
797812
cacheKey: string;
798813
queryName: string;
@@ -805,7 +820,7 @@ export class SqliteStore {
805820
}> {
806821
const db = this.ensureDb();
807822
const conditions: string[] = [];
808-
const params: Record<string, string> = {};
823+
const params: Record<string, string | number> = {};
809824

810825
if (filter?.queryName) {
811826
conditions.push('query_name = $query_name');
@@ -828,6 +843,11 @@ export class SqliteStore {
828843
}
829844
sql += ' ORDER BY created_at DESC';
830845

846+
if (filter?.limit !== undefined) {
847+
sql += ' LIMIT $limit';
848+
params.$limit = filter.limit;
849+
}
850+
831851
const stmt = db.prepare(sql);
832852
stmt.bind(params);
833853

‎server/src/tools/annotation-tools.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ function registerAnnotationListTool(server: McpServer): void {
9292
category: z.string().optional().describe('Filter by annotation category.'),
9393
entityKey: z.string().optional().describe('Filter by exact entity key.'),
9494
entityKeyPrefix: z.string().optional().describe('Filter by entity key prefix (e.g. "repo:owner/name").'),
95-
limit: z.number().optional().describe('Maximum number of results (default: 100).'),
96-
offset: z.number().optional().describe('Number of results to skip.'),
95+
limit: z.number().int().positive().optional().describe('Maximum number of results (default: 100).'),
96+
offset: z.number().int().nonnegative().optional().describe('Number of results to skip.'),
9797
},
9898
async ({ category, entityKey, entityKeyPrefix, limit, offset }) => {
9999
const store = sessionDataManager.getStore();
@@ -170,7 +170,7 @@ function registerAnnotationSearchTool(server: McpServer): void {
170170
{
171171
search: z.string().describe('Full-text search query matched against annotation content, metadata, and label (SQLite FTS MATCH syntax; use * for prefix matching).'),
172172
category: z.string().optional().describe('Restrict search to a specific category.'),
173-
limit: z.number().optional().describe('Maximum number of results (default: 50).'),
173+
limit: z.number().int().positive().optional().describe('Maximum number of results (default: 50).'),
174174
},
175175
async ({ search, category, limit }) => {
176176
const store = sessionDataManager.getStore();

‎server/src/tools/audit-tools.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ function registerAuditListFindingsTool(server: McpServer): void {
109109
{
110110
owner: z.string().describe('Repository owner.'),
111111
repo: z.string().describe('Repository name.'),
112-
limit: z.number().optional().describe('Maximum number of results.'),
112+
limit: z.number().int().positive().max(1000).optional().describe('Maximum number of results (1–1000).'),
113113
},
114114
async ({ owner, repo, limit }) => {
115115
const store = sessionDataManager.getStore();

‎server/src/tools/cache-tools.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ function registerQueryResultsCacheLookupTool(server: McpServer): void {
4444
queryName: z.string().optional().describe('Query name to search for (e.g. "PrintAST", "CallGraphFrom").'),
4545
databasePath: z.string().optional().describe('Database path to search for.'),
4646
language: z.string().optional().describe('Filter by language (e.g. "cpp", "javascript").'),
47+
limit: z.number().int().positive().max(500).optional().describe('Maximum number of cache entries to return when listing by filter (default: 50, max: 500).'),
4748
},
48-
async ({ cacheKey, queryName, databasePath, language }) => {
49+
async ({ cacheKey, queryName, databasePath, language, limit }) => {
4950
const store = sessionDataManager.getStore();
5051

5152
// Exact lookup by cache key
@@ -58,7 +59,7 @@ function registerQueryResultsCacheLookupTool(server: McpServer): void {
5859
}
5960

6061
// List matching entries
61-
const entries = store.listCacheEntries({ queryName, databasePath, language });
62+
const entries = store.listCacheEntries({ queryName, databasePath, language, limit: limit ?? 50 });
6263
if (entries.length === 0) {
6364
return { content: [{ type: 'text' as const, text: JSON.stringify({ cached: false, queryName, databasePath, language }) }] };
6465
}

‎server/test/src/lib/sqlite-store.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,23 @@ describe('SqliteStore', () => {
197197
it('should throw when deleting without a filter', () => {
198198
expect(() => store.deleteAnnotations({})).toThrow('at least one filter');
199199
});
200+
201+
it('should return empty results for invalid FTS MATCH syntax', () => {
202+
store.createAnnotation('note', 'key', 'hello world');
203+
// Trailing operator is invalid FTS syntax and would normally throw.
204+
// listAnnotations should catch it and return [] instead.
205+
const results = store.listAnnotations({ search: 'hello AND' });
206+
expect(results).toEqual([]);
207+
});
208+
209+
it('should support offset without explicit limit', () => {
210+
for (let i = 0; i < 5; i++) {
211+
store.createAnnotation('note', `key-${i}`, `content ${i}`);
212+
}
213+
// Offset only — previously would cause a SQLite syntax error.
214+
const results = store.listAnnotations({ offset: 2 });
215+
expect(results).toHaveLength(3);
216+
});
200217
});
201218

202219
describe('Persistence', () => {
@@ -412,6 +429,18 @@ describe('SqliteStore', () => {
412429
expect(store.listCacheEntries({ language: 'java' })).toHaveLength(1);
413430
});
414431

432+
it('should limit cache entries returned when limit filter is set', () => {
433+
for (let i = 0; i < 5; i++) {
434+
store.putCacheEntry({
435+
cacheKey: `k${i}`, queryName: 'Q', queryPath: '/q.ql',
436+
databasePath: '/db', language: 'cpp', codeqlVersion: '2.25.0',
437+
outputFormat: 'csv', resultContent: `c${i}`,
438+
});
439+
}
440+
const limited = store.listCacheEntries({ limit: 3 });
441+
expect(limited).toHaveLength(3);
442+
});
443+
415444
it('should clear cache entries by filter', () => {
416445
store.putCacheEntry({
417446
cacheKey: 'x', queryName: 'Q', queryPath: '/q.ql',

0 commit comments

Comments
 (0)