From aa1818e1817c6dacbb2ebfa762112e232e442755 Mon Sep 17 00:00:00 2001 From: Riley Shott Date: Wed, 13 May 2026 20:07:13 -0700 Subject: [PATCH] fix: clamp negative fromLine in get to avoid silent tail content The query tool description tells agents to compute fromLine = line - 20 for context around a hit. For hits in lines 1 through 20 that yields a negative fromLine, which propagated unchanged through: MCP get handler -> store.getDocumentBody -> Array.prototype.slice A negative slice start offsets from the end of the array rather than clamping to the beginning, so a top-of-file hit on a long document returned an empty string and on a short document returned content from the wrong region (e.g. lines 11-30 of a 30-line file in response to a request for the head of the document). The lineNumbers branch was the same shape: addLineNumbers(text, -19) emitted "-19:", "-18:" prefixes. Same buggy slice lived in the CLI getDocument path independently. Fix in three layers, plus the docstring: - src/mcp/server.ts: clamp parsedFromLine to >= 1 after parsing input args and the :line suffix, before it reaches getDocumentBody and addLineNumbers. Also tighten the query tool's recommendation to `fromLine = max(1, line - 20)` so following the docstring literally produces a valid value. - src/cli/qmd.ts: same clamp on the CLI getDocument fromLine after the colon-suffix parse. - src/store.ts: defensive Math.max(0, ...) on the slice start in getDocumentBody so SDK callers and any future entry points are protected without relying on every caller remembering to clamp. - test/store.test.ts: regression test on getDocumentBody with fromLine = -19 returns the head of the document, not the tail. - test/cli.test.ts: regression test on `qmd get --from -19` matches the no-flag baseline (head of document). --- src/cli/qmd.ts | 1 + src/mcp/server.ts | 3 ++- src/store.ts | 2 +- test/cli.test.ts | 7 +++++++ test/store.test.ts | 15 +++++++++++++++ 5 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/cli/qmd.ts b/src/cli/qmd.ts index cdbc241..8b6ada1 100755 --- a/src/cli/qmd.ts +++ b/src/cli/qmd.ts @@ -844,6 +844,7 @@ function getDocument(filename: string, fromLine?: number, maxLines?: number, lin inputPath = inputPath.slice(0, -colonMatch[0].length); } } + if (fromLine !== undefined) fromLine = Math.max(1, fromLine); const parsedIndexPath = isVirtualPath(inputPath) ? parseVirtualPath(inputPath) : null; if (parsedIndexPath?.indexName) { diff --git a/src/mcp/server.ts b/src/mcp/server.ts index a9ec99f..69c7eff 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -243,7 +243,7 @@ async function createMcpServer(store: QMDStore): Promise { title: "Query", description: `Search the knowledge base using a query document — one or more typed sub-queries combined for best recall. -Each result includes a \`line\` field with the absolute 1-indexed line of the best match in the source markdown. To read more context around a hit, call \`get(file, fromLine = line - 20, maxLines = 80, lineNumbers = true)\`. +Each result includes a \`line\` field with the absolute 1-indexed line of the best match in the source markdown. To read more context around a hit, call \`get(file, fromLine = max(1, line - 20), maxLines = 80, lineNumbers = true)\`. ## Query Types @@ -389,6 +389,7 @@ Intent-aware lex (C++ performance, not sports): parsedFromLine = parseInt(colonMatch[1], 10); lookup = lookup.slice(0, -colonMatch[0].length); } + if (parsedFromLine !== undefined) parsedFromLine = Math.max(1, parsedFromLine); const result = await store.get(lookup, { includeBody: false }); diff --git a/src/store.ts b/src/store.ts index f927ccc..003feca 100644 --- a/src/store.ts +++ b/src/store.ts @@ -3800,7 +3800,7 @@ export function getDocumentBody(db: Database, doc: DocumentResult | { filepath: let body = row.body; if (fromLine !== undefined || maxLines !== undefined) { const lines = body.split('\n'); - const start = (fromLine || 1) - 1; + const start = Math.max(0, (fromLine || 1) - 1); const end = maxLines !== undefined ? start + maxLines : lines.length; body = lines.slice(start, end).join('\n'); } diff --git a/test/cli.test.ts b/test/cli.test.ts index 2535fe4..769db00 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -542,6 +542,13 @@ describe("CLI Get Command", () => { // Should indicate file not found expect(exitCode).toBe(1); }); + + test("clamps negative --from to top of file (no silent tail content)", async () => { + const baseline = await runQmd(["get", "README.md"]); + const negative = await runQmd(["get", "README.md", "--from", "-19"]); + expect(negative.exitCode).toBe(0); + expect(negative.stdout).toBe(baseline.stdout); + }); }); describe("CLI Multi-Get Command", () => { diff --git a/test/store.test.ts b/test/store.test.ts index 6b3be5b..2adf717 100644 --- a/test/store.test.ts +++ b/test/store.test.ts @@ -1713,6 +1713,21 @@ describe("Document Retrieval", () => { expect(body).toBeNull(); await cleanupTestDb(store); }); + + test("getDocumentBody clamps negative fromLine to top of document", async () => { + const store = await createTestStore(); + const collectionName = await createTestCollection({ pwd: "/path" }); + await insertTestDocument(store.db, collectionName, { + name: "mydoc", + displayPath: "mydoc.md", + body: "Line 1\nLine 2\nLine 3\nLine 4\nLine 5", + }); + + const body = store.getDocumentBody({ filepath: "/path/mydoc.md" }, -19, 80); + expect(body).toBe("Line 1\nLine 2\nLine 3\nLine 4\nLine 5"); + + await cleanupTestDb(store); + }); }); describe("findDocuments (multi-get)", () => {