fix(mcp): bound query previews and advertise repo defaults (#3728)

* fix(mcp): reduce query payload and retry overhead

* fix(mcp): fetch bounded previews at query time

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
This commit is contained in:
Gergő Magyar 2026-10-26 08:53:47 +00:00 • committed by GitHub
parent 63f2f8e590
commit 6122657afc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 241 additions and 100 deletions

View file

@ -409,11 +409,11 @@ the paid job. Only these watchdogs have Actions write permission.
The protected `gitnexus-evolution` environment holds these four settings
(configured and verified by the live probes below):
| Setting | Kind | Purpose |
| --- | --- | --- |
| `GITNEXUS_EVOLUTION_AWS_ROLE_ARN` | Secret | AWS role assumed through GitHub OIDC |
| `GITNEXUS_EVOLUTION_EC2_INSTANCE_ID` | Secret | Dedicated runner instance |
| `GITNEXUS_EVOLUTION_AWS_REGION` | Variable | Instance region |
| Setting | Kind | Purpose |
| -------------------------------------- | -------- | ----------------------------------------------- |
| `GITNEXUS_EVOLUTION_AWS_ROLE_ARN` | Secret | AWS role assumed through GitHub OIDC |
| `GITNEXUS_EVOLUTION_EC2_INSTANCE_ID` | Secret | Dedicated runner instance |
| `GITNEXUS_EVOLUTION_AWS_REGION` | Variable | Instance region |
| `GITNEXUS_EVOLUTION_STOP_SCHEDULE_UTC` | Variable | Actual weekly EventBridge stop, `DAY HH:MM` UTC |
No long-lived AWS credential is stored. The AWS action obtains short-lived
@ -875,6 +875,25 @@ task ID, prompt, verification command, and oracle bytes produce a new task
digest. Old agent patches are not available for rescoring; keep historical
measurements unchanged and compare only runs with matching task contracts.
## MCP response cost
With `include_content: true`, queries share a 16,000-character preview allowance
equally across unique symbols on the returned page. The database returns only
those prefixes, after ranking and pagination. `content_truncated: true` marks
incomplete source; every match remains present. Use `context` for a full symbol
or `read_file` for a bounded file range. This applies to CLI, HTTP, and MCP
queries; group queries already request no source content.
MCP query guidance lives in the JSON `next` field. Explicit `maxTokens` retains
its existing final text cap, which can truncate JSON; it does not change the
query preview allowance. Payload reductions alone do not establish agent cost
or completion-time improvements: previews can require follow-up reads.
MCP tool schemas expose the configured default repository name and path. The
sandbox already configures `/workspace` as its default, so the schema identifies
`benchmark-target` without adding benchmark-specific instructions to task prompts.
Compare stable and candidate runtimes using the same harness revision.
## Relation to the SWE-bench harness
The rest of `eval/` benchmarks GitNexus _tools_ inside a litellm agent loop

View file

@ -113,16 +113,23 @@ for (const entry of cases) {
assert.equal(typeof text, 'string');
const currentText = text as string;
const hint = getNextStepHint(entry.tool, entry.args, false, entry.result);
assert.ok(currentText.endsWith(hint));
const body = hint ? currentText.slice(0, -hint.length) : currentText;
if (typeof entry.result === 'string') assert.equal(body, entry.result);
else assert.deepEqual(JSON.parse(body), entry.result);
if (entry.tool === 'query') {
const { next, ...body } = JSON.parse(currentText);
assert.equal(next, hint);
assert.deepEqual(body, entry.result);
} else {
assert.ok(currentText.endsWith(hint));
const body = hint ? currentText.slice(0, -hint.length) : currentText;
if (typeof entry.result === 'string') assert.equal(body, entry.result);
else assert.deepEqual(JSON.parse(body), entry.result);
}
const prettyText =
(typeof entry.result === 'string' ? entry.result : JSON.stringify(entry.result, null, 2)) +
hint;
const baselineBytes = Buffer.byteLength(prettyText);
const currentBytes = Buffer.byteLength(currentText);
assert.ok(currentBytes <= baselineBytes);
// Moving the query hint into JSON adds a field even for an empty result.
assert.ok(currentBytes <= baselineBytes + (entry.tool === 'query' ? 64 : 0));
measurements.push({
name: entry.name,
baselineBytes,

View file

@ -154,6 +154,7 @@ import {
QUERY_DEFAULT_MAX_SYMBOLS,
QUERY_MAX_LIMIT,
QUERY_MAX_MAX_SYMBOLS,
QUERY_CONTENT_MAX_CHARS,
CONTEXT_CHAIN_MAX_DEPTH,
CHECKOUT_SOURCE_TOOLS,
READ_FILE_DEFAULT_MAX_LINES,
@ -3516,21 +3517,10 @@ export class LocalBackend {
>();
const definitions: any[] = []; // standalone symbols not in any process
// Batch-fetch process participation, cohesion, and (optionally) content for
// ALL matched symbols in 2-3 graph queries instead of 2-3 *per symbol*. The
// previous per-symbol loop issued up to 3N sequential pool round-trips
// (searchLimit symbols × {STEP_IN_PROCESS, MEMBER_OF, content}); on a warm
// repo the IPC + query-setup overhead of those round-trips dominated query
// latency. Collapsing to `WHERE n.id IN $nodeIds` preserves identical output
// (the aggregation loop below is unchanged) while cutting the round-trips.
// Array params bind through the pool exactly as bm25Search's
// `WHERE n.id IN $nodeIds` already does. (Ported from gitnexus-enterprise
// PR #222 — N+1 → 2-3 batched queries.)
const nodeIds = merged.map(([, m]) => m.data?.nodeId).filter((id): id is string => !!id);
const processRowsByNode = new Map<string, any[]>();
const cohesionByNode = new Map<string, { cohesion: number; module?: string }>();
const contentByNode = new Map<string, string>();
// Set when a batched enrichment query throws a REAL failure (timeout, lock,
// native fault) — NOT the benign "no Process/Community table" case, which is
// a normal config (a repo analyzed without processes/communities) and must
@ -3611,30 +3601,6 @@ export class LocalBackend {
logQueryError('query:cluster-info', e);
if (!isBenignMissingTableError(e)) enrichmentDegraded = true;
}
// Optionally fetch content for every matched symbol.
if (includeContent) {
try {
const rows = await executeParameterized(
repo.lbugPath,
`
MATCH (n)
WHERE n.id IN $nodeIds
RETURN n.id AS nodeId, n.content AS content
`,
{ nodeIds: ids },
);
for (const row of rows) {
assertQueryIdentity(row, 'nodeId', 0);
const nid = row.nodeId ?? row[0];
contentByNode.set(nid, row.content ?? row[1]);
}
} catch (e) {
rethrowSymbolIdentityError(e);
logQueryError('query:content-fetch', e);
if (!isBenignMissingTableError(e)) enrichmentDegraded = true;
}
}
}
// Aggregation is unchanged from the per-symbol version — it now reads the
@ -3657,7 +3623,6 @@ export class LocalBackend {
const coh = cohesionByNode.get(sym.nodeId);
const cohesion = coh?.cohesion ?? 0;
const module = coh?.module;
const content = includeContent ? contentByNode.get(sym.nodeId) : undefined;
const symbolEntry = {
id: sym.nodeId,
@ -3667,7 +3632,6 @@ export class LocalBackend {
startLine: toDisplayLine(sym.startLine),
endLine: toDisplayLine(sym.endLine),
...(module ? { module } : {}),
...(includeContent && content ? { content } : {}),
};
if (processRows.length === 0) {
@ -3941,8 +3905,45 @@ export class LocalBackend {
maxSymbolsPerProcess,
chainByProcessId,
});
const returnedDefinitions = definitions.slice(0, 20);
timer.stop(); // formatting
if (includeContent) {
timer.start('content');
const targets = new Map<
string,
{ id: string; content?: string; content_truncated?: boolean }
>();
for (const symbol of [...process_symbols, ...returnedDefinitions]) {
if (symbol.id && !targets.has(symbol.id)) targets.set(symbol.id, symbol);
}
const previewChars = Math.floor(QUERY_CONTENT_MAX_CHARS / Math.max(1, targets.size));
for (const ids of chunk([...targets.keys()], LBUG_QUERY_BATCH_SIZE)) {
try {
const rows = await executeParameterized(
repo.lbugPath,
`MATCH (n) WHERE n.id IN $nodeIds
RETURN n.id AS nodeId, substring(n.content, 1, $previewChars) AS content,
size(n.content) > $previewChars AS contentTruncated`,
{ nodeIds: ids, previewChars },
);
for (const row of rows) {
assertQueryIdentity(row, 'nodeId', 0);
const target = targets.get(row.nodeId ?? row[0]);
if (!target) continue;
const content = row.content ?? row[1];
if (content) target.content = content;
if (row.contentTruncated ?? row[2]) target.content_truncated = true;
}
} catch (e) {
rethrowSymbolIdentityError(e);
logQueryError('query:content-fetch', e);
if (!isBenignMissingTableError(e)) enrichmentDegraded = true;
}
}
timer.stop();
}
// End-to-end wall time — deliberately a separate mark so callers can
// compare sum(phases) vs wall to see how much Promise.all concurrency
// saved. Must come before summary() so it's included.
@ -4134,7 +4135,7 @@ export class LocalBackend {
return {
processes,
process_symbols,
definitions: definitions.slice(0, 20), // cap standalone definitions
definitions: returnedDefinitions,
timing,
...(contentAvailability ? { contentAvailability } : {}),
...(warnings.length > 0 && { warning: warnings.join(' ') }),

View file

@ -333,18 +333,31 @@ export class McpRepositoryPolicy {
}
toolForMcp(tool: GitNexusTool): GitNexusTool {
if (!this.restricted) return tool;
const defaultRepo =
this.defaultRepo ??
(this.restricted && this.allowed.length === 1 ? this.allowed[0] : undefined);
if (!this.restricted && !defaultRepo) return tool;
const properties = { ...tool.inputSchema.properties };
const repo = properties.repo;
if (repo && typeof repo === 'object') {
const description = this.restricted
? 'Allowed indexed repository name or path. Group-mode values are unavailable.'
: repo.description;
properties.repo = {
...repo,
description: 'Allowed indexed repository name or path. Group-mode values are unavailable.',
description: defaultRepo
? `${description} Default repository: ${JSON.stringify(defaultRepo.name)}.`
: description,
...(defaultRepo ? { default: defaultRepo.path } : {}),
};
}
delete properties.subgroup;
delete properties.crossDepth;
const description = scrubGroupDescription(tool.description);
if (this.restricted) {
delete properties.subgroup;
delete properties.crossDepth;
}
const description = this.restricted
? scrubGroupDescription(tool.description)
: tool.description;
return { ...tool, description, inputSchema: { ...tool.inputSchema, properties } };
}

View file

@ -73,7 +73,7 @@ export function getNextStepHint(
return `\n\n---\n**Next:** READ gitnexus://repo/{name}/context for any repo above to get its overview and check staleness. If pagination.hasMore is true, call list_repos again with offset set to pagination.nextOffset to fetch the rest.`;
case 'query':
return `\n\n---\n**Next:** To understand a specific symbol in depth, use context({name: "<symbol_name>"${repoParam}}) to see categorized refs and process participation.${args?.include_content === true ? ` For source of a process_symbols row without content, use context({uid: "<id>", include_content: true${repoParam}}).` : ''}`;
return `Use context({uid: "<id>"${repoParam}}) for a symbol's references and processes.${args?.include_content === true ? ` Source previews marked content_truncated are incomplete; use context({uid: "<id>", include_content: true${repoParam}}) or read_file with a bounded line range for more.` : ''}`;
case 'context':
return `\n\n---\n**Next:** If planning changes, use impact({target: "${args?.name || '<name>'}", direction: "upstream"${repoParam}}) to check blast radius. To see execution flows, READ gitnexus://repo/${repoPath}/processes.`;
@ -284,15 +284,17 @@ export function createMCPServer(
await scopedBackend.callTool(name, backendArgs),
responseFilter,
);
// Formatting whitespace costs model input tokens; keep every value and hint.
const resultText = typeof result === 'string' ? result : JSON.stringify(result);
const hint = getNextStepHint(name, args as Record<string, any> | undefined, readOnly, result);
const resultText =
name === 'query' && result !== null && typeof result === 'object' && !Array.isArray(result)
? JSON.stringify({ ...result, next: hint })
: (typeof result === 'string' ? result : JSON.stringify(result)) + hint;
return {
content: [
{
type: 'text',
text: applyMcpMaxTokens(resultText + hint, maxTokens),
text: applyMcpMaxTokens(resultText, maxTokens),
},
],
};

View file

@ -101,6 +101,8 @@ export const QUERY_DEFAULT_MAX_SYMBOLS = 25;
/** Advertised query page maxima (schema + LocalBackend.query reject, not clamp). */
export const QUERY_MAX_LIMIT = 100;
export const QUERY_MAX_MAX_SYMBOLS = 200;
/** Source preview characters shared across unique symbols on a query result page. */
export const QUERY_CONTENT_MAX_CHARS = 16_000;
export const CONTEXT_CHAIN_MAX_DEPTH = 3;
const CWD_AWARE_REPO_OMISSION =
@ -222,8 +224,7 @@ ${HOT_READ_STALENESS_NOTE}`,
},
include_content: {
type: 'boolean',
description:
'Include source text retained for matching symbols (default: false). The response reports contentAvailability; indexes built with content retention "none" explicitly report unavailable content. Content is sent once per symbol id, on its first process_symbols row; context({uid: "<id>", include_content: true}) returns it for any row.',
description: `Include source text retained for matching symbols as previews (default: false), sharing ${QUERY_CONTENT_MAX_CHARS} characters equally across unique returned symbols per repository. content_truncated marks incomplete previews; all matches remain present. Content is sent once per symbol id, on its first process_symbols row; context({uid: "<id>", include_content: true}) returns it for any row. Use read_file line ranges for large sources. contentAvailability reports index retention. Query next is JSON guidance.`,
default: false,
},
chain_depth: {

View file

@ -96,7 +96,7 @@ describe('MCP index generation freshness', () => {
const text = (result.content as Array<{ type: string; text: string }>).find(
(item) => item.type === 'text',
)!.text;
const hint = getNextStepHint(name, args);
const hint = name === 'query' ? '' : getNextStepHint(name, args);
return JSON.parse(hint ? text.slice(0, -hint.length) : text);
}

View file

@ -9,7 +9,7 @@ import * as adapter from '../../src/core/lbug/lbug-adapter.js';
import { runFullAnalysis } from '../../src/core/run-analyze.js';
import { LocalBackend } from '../../src/mcp/local/local-backend.js';
import { SYMBOL_IDENTITY_RECOVERY_SUGGESTION } from '../../src/mcp/local/query-result-integrity.js';
import { createMCPServer, getNextStepHint } from '../../src/mcp/server.js';
import { createMCPServer } from '../../src/mcp/server.js';
import { getStoragePaths } from '../../src/storage/repo-manager.js';
import { commitAll, initGitRepo } from '../helpers/temp-git-repo.js';
import { createTempDir } from '../helpers/test-db.js';
@ -87,7 +87,12 @@ describe('query identity boundary through the real MCP server', () => {
await temp?.cleanup();
});
async function query(mutate?: () => Promise<void>, legacy = false, target = repo): Promise<any> {
async function query(
mutate?: () => Promise<void>,
legacy = false,
target = repo,
includeContent = true,
): Promise<any> {
await fs.copyFile(snapshot, lbugPath);
if (mutate) {
await adapter.initLbug(lbugPath);
@ -105,7 +110,7 @@ describe('query identity boundary through the real MCP server', () => {
const args = {
repo: target,
[legacy ? 'query' : 'search_query']: 'integrityNeedle',
include_content: true,
include_content: includeContent,
chain_depth: 1,
};
try {
@ -114,8 +119,7 @@ describe('query identity boundary through the real MCP server', () => {
expect(result.isError).not.toBe(true);
const content = result.content as Array<{ type: string; text: string }>;
const text = content.find((item) => item.type === 'text')!.text;
const hint = getNextStepHint('query', args);
return JSON.parse(hint ? text.slice(0, -hint.length) : text);
return JSON.parse(text);
} finally {
await client.close();
await server.close();
@ -140,6 +144,49 @@ describe('query identity boundary through the real MCP server', () => {
expect(result.processes.length).toBeGreaterThan(0);
});
it.each(['x'.repeat(538_895), '"\\\n😀'.repeat(30_000)])(
'bounds source previews in native queries without losing matches (%#)',
async (source) => {
const setSource = async () => {
for (const label of ['Function', 'File']) {
await adapter.executePrepared(`MATCH (n:${label}) SET n.content = $source`, { source });
}
};
const before = await query(setSource, false, repo, false);
const result = await query(setSource);
expect(result).not.toHaveProperty('error');
expect(result).not.toHaveProperty('partial');
expect(result.processes).toEqual(before.processes);
const rows = [...result.process_symbols, ...result.definitions];
expect(rows.map(({ id, process_id }) => ({ id, process_id }))).toEqual(
[...before.process_symbols, ...before.definitions].map(({ id, process_id }) => ({
id,
process_id,
})),
);
const previews = rows.filter((row) => row.content !== undefined);
expect(previews.some((row) => row.id === UID)).toBe(true);
expect(new Set(previews.map((row) => row.id)).size).toBe(previews.length);
expect(previews.reduce((sum, row) => sum + [...row.content].length, 0)).toBeLessThanOrEqual(
16_000,
);
for (const row of previews) {
expect(row.content.length).toBeGreaterThan(0);
expect(source.startsWith(row.content)).toBe(true);
expect(row.content).not.toContain('\uFFFD');
expect(row.content_truncated).toBe(true);
}
const backend = new LocalBackend();
try {
await backend.init();
const full = await backend.callTool('context', { repo, uid: UID, include_content: true });
expect(full.symbol.content).toBe(source);
} finally {
await backend.dispose();
}
},
);
it.each([
['NUL symbol name', 'MATCH (n:Function {id: $uid}) SET n.name = $value', 'invalid\0name'],
['NUL symbol path', 'MATCH (n:Function {id: $uid}) SET n.filePath = $value', 'invalid\0path'],

View file

@ -187,31 +187,44 @@ describe('createMCPServer', () => {
}
});
it('keeps repo optional when a default repo is configured', async () => {
const backend = createMockBackend({
listRepos: vi.fn().mockResolvedValue([
{ name: 'alpha', path: '/tmp/alpha' },
{ name: 'beta', path: '/tmp/beta' },
]),
});
const repositoryPolicy = await createMcpRepositoryPolicy(backend, {
GITNEXUS_MCP_DEFAULT_REPO: 'alpha',
});
const server = createMCPServer(backend, { repositoryPolicy });
const client = new Client({ name: 'default-repo-client', version: '0.0.0' });
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
it.each([{ GITNEXUS_MCP_DEFAULT_REPO: 'alpha' }, { GITNEXUS_MCP_ALLOWED_REPOS: 'alpha' }])(
'advertises the resolved repository default in tool schemas (%j)',
async (env) => {
const backend = createMockBackend({
listRepos: vi.fn().mockResolvedValue([
{ name: 'alpha', path: '/tmp/alpha' },
{ name: 'beta', path: '/tmp/beta' },
]),
});
const repositoryPolicy = await createMcpRepositoryPolicy(backend, env);
const server = createMCPServer(backend, { repositoryPolicy });
const client = new Client({ name: 'default-repo-client', version: '0.0.0' });
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
try {
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
const tools = await client.listTools();
const query = tools.tools.find((tool) => tool.name === 'query');
try {
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
const tools = await client.listTools();
const query = tools.tools.find((tool) => tool.name === 'query');
expect(query?.inputSchema.required).not.toContain('repo');
} finally {
await client.close();
await server.close();
}
});
expect(query?.inputSchema.required).not.toContain('repo');
expect(query?.inputSchema.properties?.repo).toMatchObject({
default: '/tmp/alpha',
description: expect.stringContaining('"alpha"'),
});
await client.callTool({ name: 'query', arguments: { search_query: 'auth' } });
expect(backend.callTool).toHaveBeenLastCalledWith('query', {
search_query: 'auth',
repo: '/tmp/alpha',
});
expect(
GITNEXUS_TOOLS.find((tool) => tool.name === 'query')?.inputSchema.properties.repo,
).not.toHaveProperty('default');
} finally {
await client.close();
await server.close();
}
},
);
it('requires repo when multiple allowed repositories are visible without a default', async () => {
const backend = createMockBackend({
@ -261,9 +274,9 @@ describe('getNextStepHint (via tool call response)', () => {
});
const QUERY_HINT_BASE =
'\n\n---\n**Next:** To understand a specific symbol in depth, use context({name: "<symbol_name>", repo: "demo"}) to see categorized refs and process participation.';
'Use context({uid: "<id>", repo: "demo"}) for a symbol\'s references and processes.';
it('query hint is unchanged when include_content is not set', async () => {
it('query hint uses the returned identity and repository', async () => {
const backend = createMockBackend({
callTool: vi.fn().mockResolvedValue({ processes: [] }),
});
@ -271,8 +284,8 @@ describe('getNextStepHint (via tool call response)', () => {
search_query: 'auth',
repo: 'demo',
});
expect(text.endsWith(QUERY_HINT_BASE)).toBe(true);
expect(text).not.toContain('context({uid:');
expect(JSON.parse(text).next).toBe(QUERY_HINT_BASE);
expect(text).not.toContain('content_truncated');
});
it('query hint names the context({uid, include_content}) fallback when include_content is true', async () => {
@ -284,11 +297,9 @@ describe('getNextStepHint (via tool call response)', () => {
repo: 'demo',
include_content: true,
});
expect(
text.endsWith(
`${QUERY_HINT_BASE} For source of a process_symbols row without content, use context({uid: "<id>", include_content: true, repo: "demo"}).`,
),
).toBe(true);
expect(JSON.parse(text).next).toBe(
`${QUERY_HINT_BASE} Source previews marked content_truncated are incomplete; use context({uid: "<id>", include_content: true, repo: "demo"}) or read_file with a bounded line range for more.`,
);
});
it('grep hint converts the 1-based hit line to a 0-based read_file window', async () => {
@ -384,7 +395,45 @@ describe('getNextStepHint rename', () => {
});
describe('MCP output budgets', () => {
it('leaves the complete formatted response unchanged when no budget is configured', async () => {
it('serializes query previews and diagnostics as one JSON document', async () => {
const result = {
processes: [{ id: 'process-1', symbol_count: 2 }],
process_symbols: [
{ id: 'Function:small', process_id: 'process-1', content: 'function small() {}' },
{
id: 'Function:large',
process_id: 'process-1',
content: 'large preview',
content_truncated: true,
},
],
definitions: [
{
id: 'File:large.ts',
filePath: 'large.ts',
content: 'file preview',
content_truncated: true,
},
],
warning: 'Some keyword indexes are unavailable',
partial: true,
staleness: { status: 'behind', commitsBehind: 2 },
contentAvailability: 'available',
};
const backend = createMockBackend({ callTool: vi.fn().mockResolvedValue(result) });
const { text, isError } = await callToolThroughServer(backend, 'query', {
search_query: 'large',
include_content: true,
});
expect(isError).toBe(false);
const { next, ...body } = JSON.parse(text);
expect(body).toEqual(result);
expect(next).toContain('read_file');
expect(result).not.toHaveProperty('next');
});
it('preserves small query data and guidance without source omissions', async () => {
const previous = process.env.GITNEXUS_MCP_DEFAULT_MAX_TOKENS;
delete process.env.GITNEXUS_MCP_DEFAULT_MAX_TOKENS;
try {
@ -395,8 +444,10 @@ describe('MCP output budgets', () => {
search_query: 'auth',
});
expect(isError).toBe(false);
expect(text).toContain('"payload":"complete"');
expect(text).toContain('**Next:**');
expect(JSON.parse(text)).toMatchObject({
payload: 'complete',
next: expect.stringContaining('context'),
});
expect(text.endsWith('\n…')).toBe(false);
} finally {
if (previous === undefined) delete process.env.GITNEXUS_MCP_DEFAULT_MAX_TOKENS;