mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-05 02:43:32 +00:00
perf(mcp): avoid O(n) git spawns on tools/list with many repos (#3259)
* perf(mcp): avoid O(n) git spawns on tools/list with many repos toolSchemaRepoRequirements called listAllowedRepos -> listRepos -> checkStalenessAsync for every registered repo. With ~200 repos, this spawned 200 parallel git rev-list processes on every tools/list discovery call, causing a ~30s delay. Replaced with a lightweight countRepos() method that reads the registry file once without spawning git processes, preserving full staleness checks for list_repos. * Address PR review feedback (#3259) Count the validated registry in countRepos so tools/list cannot advertise a multi-repo schema for ENOENT ghosts, and update the unrestricted listTools mocks to that contract. Note: pre-existing failure in update-notice.test.ts (missing dist/cli/mcp.js) not addressed by this PR. Co-authored-by: Cursor <cursoragent@cursor.com> * Add a 200-repo tools/list bench for the countRepos path (#3259) Pin the #1363 comparison (listRepos git fan-out vs validated countRepos / listTools) in-tree so the latency claim can be re-run. Also drop the change-history comments on the unrestricted schema arm. Co-authored-by: Cursor <cursoragent@cursor.com> * Gate the tools/list bench with baselines and CI --check (#3259) Exact registry/schema floors plus ratio timing, no millisecond ceiling, so restoring listRepos() on tools/list fails CI. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3259) Align unrestricted tools/list schema flags with the refreshed registry snapshot, and make the bench reject a non-positive BENCH_REPS and isolate fixtures by N. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * Address PR review feedback (#3259) Isolate the tools/list bench from GITNEXUS_MCP_READ_ONLY and create the default fixture under mkdtempSync so CodeQL is not looking at a predictable /tmp path. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This commit is contained in:
parent
7bbaf6b73b
commit
d1a3edd333
10 changed files with 519 additions and 12 deletions
11
.github/workflows/ci-tests.yml
vendored
11
.github/workflows/ci-tests.yml
vendored
|
|
@ -608,6 +608,17 @@ jobs:
|
|||
run: node --import tsx bench/python-workspace-import-scan/measure.mjs --check
|
||||
working-directory: gitnexus
|
||||
|
||||
- name: MCP tools/list countRepos vs listRepos guards (#3259, #3184)
|
||||
if: ${{ !cancelled() }}
|
||||
# Build-free: exact registry cardinality + tool-roster + schema-flag
|
||||
# floors, then ratio timing only (countRepos/listRepos and
|
||||
# listTools/listRepos). No millisecond ceiling — this repo has
|
||||
# already been bitten by a fixed ms budget. Isolated GITNEXUS_HOME;
|
||||
# fixture is N real git repos so listRepos pays rev-list. See
|
||||
# bench/mcp-tools-list/measure.mjs.
|
||||
run: node --import tsx bench/mcp-tools-list/measure.mjs --check
|
||||
working-directory: gitnexus
|
||||
|
||||
- name: C++ qualified-namespace resolution guards (#2788)
|
||||
if: ${{ !cancelled() }}
|
||||
# Build-free: asserts resolveCppQualifiedNamespaceMember resolves an
|
||||
|
|
|
|||
35
gitnexus/bench/mcp-tools-list/baselines.json
Normal file
35
gitnexus/bench/mcp-tools-list/baselines.json
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
{
|
||||
"_what": "Baselines for bench/mcp-tools-list/measure.mjs --check. Guards unrestricted MCP tools/list after #3259: validated registry cardinality (countRepos) instead of listRepos() staleness git. Same approach as bench/parse-dispatch-rounds and bench/python-workspace-import-scan — exact floors first; the only timing arms are ratios. Never a millisecond ceiling.",
|
||||
|
||||
"_triage": "READ THIS BEFORE RE-RUNNING. n_repos, count_repos, list_repos, tools_listed, schema_read_only_requires_repo and schema_mutating_requires_repo are DETERMINISTIC: a re-run never changes them, and none may be re-baselined to make CI green. count_vs_listRepos_ratio and listTools_vs_listRepos_ratio are UPPER timing arms. Runner contention dominates both, so re-run on an idle machine before investigating and read the reported `reps` first. If exactly one arm fails and it is a timing arm, suspect the machine. If listRepos() itself stops paying git (unrelated change), both ratios move toward 1 and need an explained re-baseline — do not just raise the budget.",
|
||||
|
||||
"n_repos": 200,
|
||||
"count_repos": 200,
|
||||
"list_repos": 200,
|
||||
"_shape_note": "THE FLOOR. Without these three, every ratio below is a ceiling over nothing. listTools_vs_listRepos_ratio only asserts something while the corpus still pays N parallel rev-list processes. Shrink it to three happy-path rows and both arms are cheap; the ratio still passes, asserting a property the corpus no longer has.",
|
||||
|
||||
"tools_listed": 17,
|
||||
"_tools_note": "Exact GITNEXUS_TOOLS roster size returned by client.listTools(). A schema path that throws, filters, or returns [] still looks fast on the ratio arm.",
|
||||
|
||||
"schema_read_only_requires_repo": true,
|
||||
"schema_mutating_requires_repo": true,
|
||||
"_schema_note": "This unrestricted 200-repo fixture has no cwd default, so both flags must stay true. Skipping the cwd probe and advertising a single-repo schema would pass every timing arm.",
|
||||
|
||||
"count_vs_listRepos_budget": 0.15,
|
||||
"_count_ratio_note": "countRepos_ms / listRepos_ms. A RATIO rather than a millisecond ceiling, deliberately: wall-clock is runner-speed-dependent, and this repo has already been bitten by a fixed ms budget. Putting staleness git back on countRepos collapses this toward 1. Budget is 0.15 — more than 10x above the measured ~0.01, same fail-closed presence check as import-target. min-of-7 estimator.",
|
||||
|
||||
"listTools_vs_listRepos_budget": 0.75,
|
||||
"_listTools_ratio_note": "client.listTools()_ms / listRepos_ms. Restoring listRepos() on toolSchemaRepoRequirements collapses this toward 1+. Budget is 0.75 — listRepos is git-bound and can get relatively faster on GitHub-hosted runners than the Node-bound listTools arm (measured ~0.22–0.28 here). The collapse-to-1 regression still fails. min-of-7 estimator.",
|
||||
|
||||
"_measured": {
|
||||
"count_vs_listRepos_ratio": 0.026,
|
||||
"count_vs_listRepos_ratio_samples": [0.025, 0.025, 0.026, 0.026, 0.025],
|
||||
"listTools_vs_listRepos_ratio": 0.283,
|
||||
"listTools_vs_listRepos_ratio_samples": [0.242, 0.242, 0.223, 0.267, 0.234, 0.283],
|
||||
"count_ms": 8.99,
|
||||
"listRepos_ms": 349.63,
|
||||
"listTools_ms": 99.04,
|
||||
"reps": 7
|
||||
},
|
||||
"_measured_note": "Maxima (and sample lists) over consecutive local runs using the min-of-7 estimator, including one --check pass. Milliseconds are diagnostic context only — nothing gates on them."
|
||||
}
|
||||
338
gitnexus/bench/mcp-tools-list/measure.mjs
Normal file
338
gitnexus/bench/mcp-tools-list/measure.mjs
Normal file
|
|
@ -0,0 +1,338 @@
|
|||
/**
|
||||
* Build-free bench for unrestricted MCP `tools/list` with many registered
|
||||
* repos (#3259 / #3184 / #1363).
|
||||
*
|
||||
* WHY THIS EXISTS. `toolSchemaRepoRequirements` used to call
|
||||
* `listAllowedRepos()` → `listRepos()` → one `git rev-list` per registry row
|
||||
* (`checkStalenessAsync`). #1363 made that fan-out parallel (~50 s serial →
|
||||
* <1 s). #3259 removes it from schema introspection: `countRepos()` reads the
|
||||
* validated registry (`fs.access`, no git). Staleness git stays on
|
||||
* `list_repos`. Graph output and unit tests cannot see "we stopped spawning
|
||||
* git on tools/list" — putting `listRepos()` back still returns the same
|
||||
* tool roster and the same required-repo flags.
|
||||
*
|
||||
* ARMS:
|
||||
*
|
||||
* - `n_repos` / `count_repos` / `list_repos` — EXACT, and they are the FLOOR.
|
||||
* The ratio arms only mean something while the corpus still pays N parallel
|
||||
* `rev-list`s. Shrink it to three rows and both sides are cheap; the ratio
|
||||
* can still pass while the property the bench claims to guard is gone.
|
||||
*
|
||||
* - `tools_listed` — EXACT. `listTools` must still return the full
|
||||
* `GITNEXUS_TOOLS` roster. A schema path that errors or filters the set
|
||||
* would otherwise hide behind a "fast" timing arm.
|
||||
*
|
||||
* - `schema_read_only_requires_repo` / `schema_mutating_requires_repo` —
|
||||
* EXACT. On this unrestricted N-repo fixture there is no cwd default, so
|
||||
* both flags must stay true. Skipping the cwd probe and advertising a
|
||||
* single-repo schema would pass every timing arm.
|
||||
*
|
||||
* - `count_vs_listRepos_ratio` — UPPER timing arm, a RATIO not a millisecond
|
||||
* ceiling. `countRepos_ms / listRepos_ms`. Putting staleness git back on
|
||||
* `countRepos` collapses this toward 1. Wall-clock is runner-speed-
|
||||
* dependent; this repo has already been bitten by a fixed ms budget.
|
||||
*
|
||||
* - `listTools_vs_listRepos_ratio` — UPPER timing arm. The user-visible
|
||||
* `tools/list` path over the old `listRepos()` hot path. Restoring
|
||||
* `listRepos()` on schema introspection collapses this toward 1+.
|
||||
*
|
||||
* Isolated `GITNEXUS_HOME` — never touches `~/.gitnexus`. Also clears
|
||||
* `GITNEXUS_MCP_ALLOWED_REPOS`, `GITNEXUS_MCP_DEFAULT_REPO`, and
|
||||
* `GITNEXUS_MCP_READ_ONLY` so the invoking shell cannot shrink the roster.
|
||||
* Each fixture row is a real git repo whose `lastCommit` matches HEAD, so
|
||||
* `listRepos()` pays `rev-list` instead of failing open. The default root is
|
||||
* `mkdtempSync`; set `BENCH_ROOT` to reuse a tree across local runs.
|
||||
*
|
||||
* Usage:
|
||||
* node --import tsx bench/mcp-tools-list/measure.mjs
|
||||
* node --import tsx bench/mcp-tools-list/measure.mjs --check
|
||||
* BENCH_REPOS=3 node --import tsx bench/mcp-tools-list/measure.mjs # report only
|
||||
*/
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { performance } from 'node:perf_hooks';
|
||||
|
||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
|
||||
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
|
||||
|
||||
import { LocalBackend } from '../../src/mcp/local/local-backend.ts';
|
||||
import { createMcpRepositoryPolicy } from '../../src/mcp/repository-policy.ts';
|
||||
import { createMCPServer } from '../../src/mcp/server.ts';
|
||||
import { GITNEXUS_TOOLS } from '../../src/mcp/tools.ts';
|
||||
import { listRegisteredRepos } from '../../src/storage/repo-manager.ts';
|
||||
|
||||
const baselines = JSON.parse(readFileSync(new URL('./baselines.json', import.meta.url), 'utf8'));
|
||||
|
||||
const CHECK = process.argv.includes('--check');
|
||||
const PINNED_REPS = 7;
|
||||
|
||||
function positiveInt(value, fallback) {
|
||||
const n = Number(value);
|
||||
return Number.isInteger(n) && n > 0 ? n : fallback;
|
||||
}
|
||||
|
||||
const REPS = CHECK ? PINNED_REPS : positiveInt(process.env.BENCH_REPS, PINNED_REPS);
|
||||
const N = CHECK ? baselines.n_repos : positiveInt(process.env.BENCH_REPOS, baselines.n_repos);
|
||||
const ROOT =
|
||||
process.env.BENCH_ROOT ?? mkdtempSync(path.join(os.tmpdir(), 'gn-mcp-tools-list-bench-'));
|
||||
const WORK = path.join(ROOT, `n-${N}`);
|
||||
const HOME = path.join(WORK, 'home');
|
||||
|
||||
process.env.GITNEXUS_HOME = HOME;
|
||||
delete process.env.GITNEXUS_MCP_ALLOWED_REPOS;
|
||||
delete process.env.GITNEXUS_MCP_DEFAULT_REPO;
|
||||
delete process.env.GITNEXUS_MCP_READ_ONLY;
|
||||
|
||||
function git(cwd, args) {
|
||||
return execFileSync('git', args, {
|
||||
cwd,
|
||||
encoding: 'utf8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
env: {
|
||||
...process.env,
|
||||
GIT_AUTHOR_NAME: 'bench',
|
||||
GIT_AUTHOR_EMAIL: 'bench@example.com',
|
||||
GIT_COMMITTER_NAME: 'bench',
|
||||
GIT_COMMITTER_EMAIL: 'bench@example.com',
|
||||
},
|
||||
}).trim();
|
||||
}
|
||||
|
||||
function setupFixture() {
|
||||
mkdirSync(HOME, { recursive: true });
|
||||
const marker = path.join(WORK, 'ready');
|
||||
// Reuse only when the caller pinned BENCH_ROOT. The default root is a
|
||||
// mkdtempSync directory, so the ready marker cannot alias a previous run
|
||||
// and there is no exists-then-write race on a predictable /tmp name.
|
||||
if (
|
||||
process.env.BENCH_ROOT &&
|
||||
existsSync(marker) &&
|
||||
existsSync(path.join(HOME, 'registry.json'))
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const entries = [];
|
||||
for (let i = 0; i < N; i++) {
|
||||
const repoPath = path.join(WORK, 'repos', `r${i}`);
|
||||
const storagePath = path.join(repoPath, '.gitnexus');
|
||||
mkdirSync(storagePath, { recursive: true });
|
||||
writeFileSync(path.join(repoPath, 'f.txt'), `${i}\n`);
|
||||
git(repoPath, ['init', '-b', 'main']);
|
||||
git(repoPath, ['add', 'f.txt']);
|
||||
git(repoPath, ['commit', '-m', 'init']);
|
||||
entries.push({
|
||||
name: `r${i}`,
|
||||
path: repoPath,
|
||||
storagePath,
|
||||
indexedAt: '2026-09-11T00:00:00.000Z',
|
||||
lastCommit: git(repoPath, ['rev-parse', 'HEAD']),
|
||||
stats: { files: 1, nodes: 1, edges: 0, communities: 0, processes: 0 },
|
||||
});
|
||||
writeFileSync(path.join(storagePath, 'gitnexus.json'), '{}\n');
|
||||
}
|
||||
writeFileSync(path.join(HOME, 'registry.json'), `${JSON.stringify(entries, null, 2)}\n`);
|
||||
writeFileSync(marker, `${N}\n`);
|
||||
}
|
||||
|
||||
async function fastest(fn, reps) {
|
||||
await fn();
|
||||
let best = Infinity;
|
||||
let last;
|
||||
for (let r = 0; r < reps; r++) {
|
||||
const t0 = performance.now();
|
||||
last = await fn();
|
||||
best = Math.min(best, performance.now() - t0);
|
||||
}
|
||||
return { ms: best, last };
|
||||
}
|
||||
|
||||
async function listToolsOnce(backend) {
|
||||
const repositoryPolicy = await createMcpRepositoryPolicy(backend);
|
||||
const server = createMCPServer(backend, { repositoryPolicy });
|
||||
const client = new Client({ name: 'bench', version: '0.0.0' });
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
|
||||
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
|
||||
try {
|
||||
const listed = await client.listTools();
|
||||
return listed.tools.length;
|
||||
} finally {
|
||||
await client.close();
|
||||
await server.close();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A missing budget is a DELETED GATE, not a passing arm: `got > undefined` is
|
||||
* false for every measurement. `Number.isFinite` rather than `typeof ===
|
||||
* 'number'` — JSON cannot express NaN, and the stricter check is the one whose
|
||||
* name says what the gate needs.
|
||||
*/
|
||||
function requireNumeric(key) {
|
||||
const value = baselines[key];
|
||||
if (Number.isFinite(value)) return value;
|
||||
return {
|
||||
missing: `no numeric ${key} in baselines.json — a missing budget is a DELETED GATE, not a passing arm: the comparison it gates is false for every possible measurement. Deterministic: a re-run will not change it.`,
|
||||
};
|
||||
}
|
||||
|
||||
function requireBoolean(key) {
|
||||
const value = baselines[key];
|
||||
if (typeof value === 'boolean') return value;
|
||||
return {
|
||||
missing: `no boolean ${key} in baselines.json — a missing exact floor is a DELETED GATE, not a passing arm. Deterministic: a re-run will not change it.`,
|
||||
};
|
||||
}
|
||||
|
||||
setupFixture();
|
||||
|
||||
const backend = new LocalBackend();
|
||||
await backend.init();
|
||||
const policy = await createMcpRepositoryPolicy(backend);
|
||||
|
||||
const countTimed = await fastest(() => backend.countRepos(), REPS);
|
||||
const listTimed = await fastest(() => backend.listRepos(), REPS);
|
||||
const listToolsTimed = await fastest(() => listToolsOnce(backend), REPS);
|
||||
const schema = await policy.toolSchemaRepoRequirements(backend);
|
||||
const rawRegistry = await listRegisteredRepos({ validate: false });
|
||||
|
||||
const countRepos = countTimed.last;
|
||||
const listRepos = listTimed.last.length;
|
||||
const toolsListed = listToolsTimed.last;
|
||||
const countMs = countTimed.ms;
|
||||
const listReposMs = listTimed.ms;
|
||||
const listToolsMs = listToolsTimed.ms;
|
||||
const countRatio = listReposMs === 0 ? Infinity : countMs / listReposMs;
|
||||
const listToolsRatio = listReposMs === 0 ? Infinity : listToolsMs / listReposMs;
|
||||
|
||||
console.log(`n_repos : ${N} (expect ${baselines.n_repos})`);
|
||||
console.log(`count_repos : ${countRepos} (expect ${baselines.count_repos})`);
|
||||
console.log(`list_repos : ${listRepos} (expect ${baselines.list_repos})`);
|
||||
console.log(
|
||||
`tools_listed : ${toolsListed} (expect ${baselines.tools_listed}; GITNEXUS_TOOLS ${GITNEXUS_TOOLS.length})`,
|
||||
);
|
||||
console.log(
|
||||
`schema_read_only_requires_repo : ${schema.readOnlyRequiresRepo} (expect ${baselines.schema_read_only_requires_repo})`,
|
||||
);
|
||||
console.log(
|
||||
`schema_mutating_requires_repo : ${schema.mutatingRequiresRepo} (expect ${baselines.schema_mutating_requires_repo})`,
|
||||
);
|
||||
console.log(
|
||||
`count_vs_listRepos_ratio : ${countRatio.toFixed(3)} (budget <= ${baselines.count_vs_listRepos_budget})`,
|
||||
);
|
||||
console.log(
|
||||
`listTools_vs_listRepos_ratio : ${listToolsRatio.toFixed(3)} (budget <= ${baselines.listTools_vs_listRepos_budget})`,
|
||||
);
|
||||
console.log(
|
||||
`reps : ${REPS} count ${countMs.toFixed(2)}ms / listRepos ${listReposMs.toFixed(2)}ms / listTools ${listToolsMs.toFixed(2)}ms / rawRegistry ${rawRegistry.length}`,
|
||||
);
|
||||
|
||||
if (!CHECK) process.exit(0);
|
||||
|
||||
if (process.env.BENCH_REPOS && Number(process.env.BENCH_REPOS) !== baselines.n_repos) {
|
||||
console.error(
|
||||
`\nFAIL BENCH_REPOS=${process.env.BENCH_REPOS} is ignored under --check.\n` +
|
||||
` n_repos is pinned in baselines.json (${baselines.n_repos}). A smaller\n` +
|
||||
` corpus makes both timing arms cheap and the ratios stop measuring git.`,
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
let failed = false;
|
||||
|
||||
function fail(message) {
|
||||
failed = true;
|
||||
console.error(`\nFAIL ${message}`);
|
||||
}
|
||||
|
||||
const nReposBudget = requireNumeric('n_repos');
|
||||
const countBudget = requireNumeric('count_repos');
|
||||
const listBudget = requireNumeric('list_repos');
|
||||
const toolsBudget = requireNumeric('tools_listed');
|
||||
const countRatioBudget = requireNumeric('count_vs_listRepos_budget');
|
||||
const listToolsRatioBudget = requireNumeric('listTools_vs_listRepos_budget');
|
||||
const readOnlyExpect = requireBoolean('schema_read_only_requires_repo');
|
||||
const mutatingExpect = requireBoolean('schema_mutating_requires_repo');
|
||||
|
||||
for (const got of [
|
||||
nReposBudget,
|
||||
countBudget,
|
||||
listBudget,
|
||||
toolsBudget,
|
||||
countRatioBudget,
|
||||
listToolsRatioBudget,
|
||||
readOnlyExpect,
|
||||
mutatingExpect,
|
||||
]) {
|
||||
if (got && typeof got === 'object' && 'missing' in got) fail(got.missing);
|
||||
}
|
||||
|
||||
if (Number.isFinite(nReposBudget) && N !== nReposBudget) {
|
||||
fail(
|
||||
`n_repos: ${N}, expected exactly ${nReposBudget}.\n` +
|
||||
` THE FLOOR. Ratio arms only assert something while the corpus still\n` +
|
||||
` pays N parallel rev-list processes.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (Number.isFinite(countBudget) && countRepos !== countBudget) {
|
||||
fail(`count_repos: ${countRepos}, expected exactly ${countBudget}.`);
|
||||
}
|
||||
|
||||
if (Number.isFinite(listBudget) && listRepos !== listBudget) {
|
||||
fail(`list_repos: ${listRepos}, expected exactly ${listBudget}.`);
|
||||
}
|
||||
|
||||
if (Number.isFinite(toolsBudget)) {
|
||||
if (GITNEXUS_TOOLS.length !== toolsBudget) {
|
||||
fail(
|
||||
`GITNEXUS_TOOLS.length: ${GITNEXUS_TOOLS.length}, expected exactly ${toolsBudget}.\n` +
|
||||
` The tool roster moved. Explain it; do not re-baseline tools_listed alone.`,
|
||||
);
|
||||
}
|
||||
if (toolsListed !== toolsBudget) {
|
||||
fail(
|
||||
`tools_listed: ${toolsListed}, expected exactly ${toolsBudget}.\n` +
|
||||
` listTools dropped or padded the roster. A fast arm that returns [] still\n` +
|
||||
` looks like a win on the ratio.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof readOnlyExpect === 'boolean' && schema.readOnlyRequiresRepo !== readOnlyExpect) {
|
||||
fail(
|
||||
`schema_read_only_requires_repo: ${schema.readOnlyRequiresRepo}, expected ${readOnlyExpect}.\n` +
|
||||
` On this unrestricted N-repo fixture there is no cwd default. Advertising\n` +
|
||||
` a single-repo schema skips the multi-repo arm the timing ratios guard.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (typeof mutatingExpect === 'boolean' && schema.mutatingRequiresRepo !== mutatingExpect) {
|
||||
fail(
|
||||
`schema_mutating_requires_repo: ${schema.mutatingRequiresRepo}, expected ${mutatingExpect}.\n` +
|
||||
` On this unrestricted N-repo fixture there is no cwd default.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (Number.isFinite(countRatioBudget) && countRatio > countRatioBudget) {
|
||||
fail(
|
||||
`count_vs_listRepos_ratio: ${countRatio.toFixed(3)} exceeds ${countRatioBudget}.\n` +
|
||||
` countRepos should stay far cheaper than listRepos. A collapse toward 1.0\n` +
|
||||
` usually means staleness git is back on the count path.\n` +
|
||||
` Re-run on an idle machine before investigating, and check \`reps\` first.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (Number.isFinite(listToolsRatioBudget) && listToolsRatio > listToolsRatioBudget) {
|
||||
fail(
|
||||
`listTools_vs_listRepos_ratio: ${listToolsRatio.toFixed(3)} exceeds ${listToolsRatioBudget}.\n` +
|
||||
` tools/list should stay cheaper than the old listRepos() hot path. A\n` +
|
||||
` collapse toward 1.0+ usually means schema introspection calls listRepos\n` +
|
||||
` again. Re-run on an idle machine before investigating, and check \`reps\`.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (failed) process.exit(1);
|
||||
console.log('\nOK — within budget.');
|
||||
|
|
@ -2491,6 +2491,30 @@ export class LocalBackend {
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Lightweight registry count for schema-introspection callers that only
|
||||
* need to know "one repo or many?" without paying the full staleness fan-out
|
||||
* cost that listRepos() incurs. Uses the same validated registry
|
||||
* `refreshRepos` / `selectToolRepository` see (`validate: true` prunes
|
||||
* entries whose metadata is provably gone) so tools/list cannot advertise a
|
||||
* multi-repo schema for ENOENT ghosts. No git processes are spawned.
|
||||
*/
|
||||
async countRepos(): Promise<number> {
|
||||
const entries = await listRegisteredRepos({ validate: true });
|
||||
return entries.length;
|
||||
}
|
||||
|
||||
/**
|
||||
* In-memory validated registry size after the last `refreshRepos` / init /
|
||||
* `selectToolRepository` refresh. `countRepos()` does not populate this map.
|
||||
* Schema introspection uses this after a refreshed cwd probe so cardinality
|
||||
* and the probe share one snapshot — without putting `refreshRepos()` (and
|
||||
* its kuzu cleanup) on the 0–1 `countRepos` path.
|
||||
*/
|
||||
cachedRepoCount(): number {
|
||||
return this.repos.size;
|
||||
}
|
||||
|
||||
/**
|
||||
* Paginated view over {@link listRepos} for the `list_repos` MCP tool (#2119).
|
||||
*
|
||||
|
|
|
|||
|
|
@ -198,25 +198,27 @@ export class McpRepositoryPolicy {
|
|||
};
|
||||
}
|
||||
|
||||
// One fresh listing supplies both schema decisions. Besides keeping the
|
||||
// advertised contract internally consistent, this avoids doing two full
|
||||
// per-repo staleness fan-outs for every tools/list request.
|
||||
const visibleRepos = await this.listAllowedRepos(backend);
|
||||
if (visibleRepos.length <= 1) {
|
||||
// Validated registry cardinality only — no listRepos() staleness git.
|
||||
// The 0–1 arm stays a cheap countRepos() (no refreshRepos / kuzu cleanup).
|
||||
const repoCount = await backend.countRepos();
|
||||
if (repoCount <= 1) {
|
||||
return { readOnlyRequiresRepo: false, mutatingRequiresRepo: false };
|
||||
}
|
||||
try {
|
||||
// listAllowedRepos() refreshed this backend immediately above. Resolve
|
||||
// against that exact cache snapshot instead of racing another registry
|
||||
// read; only read-only schemas may advertise the cwd-derived default.
|
||||
// countRepos() does not refresh the backend; this cwd probe must.
|
||||
await backend.selectToolRepository(undefined, undefined, {
|
||||
allowCwdDefault: true,
|
||||
refreshRegistry: false,
|
||||
refreshRegistry: true,
|
||||
});
|
||||
return { readOnlyRequiresRepo: false, mutatingRequiresRepo: true };
|
||||
} catch {
|
||||
return { readOnlyRequiresRepo: true, mutatingRequiresRepo: true };
|
||||
}
|
||||
// The probe just refreshed. If that snapshot is now a singleton, match the
|
||||
// <=1 arm rather than advertising a split mutating-only schema.
|
||||
if (backend.cachedRepoCount() <= 1) {
|
||||
return { readOnlyRequiresRepo: false, mutatingRequiresRepo: false };
|
||||
}
|
||||
return { readOnlyRequiresRepo: false, mutatingRequiresRepo: true };
|
||||
}
|
||||
|
||||
private async listReposPage(
|
||||
|
|
|
|||
|
|
@ -51,6 +51,7 @@ function mockBackend() {
|
|||
: { ok: true },
|
||||
),
|
||||
listRepos: vi.fn().mockResolvedValue([]),
|
||||
countRepos: vi.fn().mockResolvedValue(0),
|
||||
resolveRepo: vi
|
||||
.fn()
|
||||
.mockResolvedValue({ name: 'test', repoPath: '/tmp/test', lastCommit: 'abc' }),
|
||||
|
|
|
|||
|
|
@ -294,6 +294,78 @@ describe('LocalBackend.init', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('LocalBackend.countRepos', () => {
|
||||
let backend: LocalBackend;
|
||||
|
||||
beforeEach(() => {
|
||||
backend = new LocalBackend();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('counts the validated registry, ignoring raw ENOENT ghost entries', async () => {
|
||||
(listRegisteredRepos as any).mockImplementation(async (opts?: { validate?: boolean }) =>
|
||||
opts?.validate
|
||||
? [MOCK_REPO_ENTRY]
|
||||
: [
|
||||
MOCK_REPO_ENTRY,
|
||||
{
|
||||
...MOCK_REPO_ENTRY,
|
||||
name: 'ghost-project',
|
||||
path: '/tmp/ghost-project',
|
||||
storagePath: '/tmp/.gitnexus/ghost-project',
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
await expect(backend.countRepos()).resolves.toBe(1);
|
||||
expect(listRegisteredRepos).toHaveBeenCalledWith({ validate: true });
|
||||
});
|
||||
|
||||
it('returns 0 when every registry row is a ghost', async () => {
|
||||
(listRegisteredRepos as any).mockImplementation(async (opts?: { validate?: boolean }) =>
|
||||
opts?.validate
|
||||
? []
|
||||
: [
|
||||
{
|
||||
...MOCK_REPO_ENTRY,
|
||||
name: 'ghost-a',
|
||||
path: '/tmp/ghost-a',
|
||||
storagePath: '/tmp/.gitnexus/ghost-a',
|
||||
},
|
||||
{
|
||||
...MOCK_REPO_ENTRY,
|
||||
name: 'ghost-b',
|
||||
path: '/tmp/ghost-b',
|
||||
storagePath: '/tmp/.gitnexus/ghost-b',
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
await expect(backend.countRepos()).resolves.toBe(0);
|
||||
expect(listRegisteredRepos).toHaveBeenCalledWith({ validate: true });
|
||||
});
|
||||
|
||||
it('reports the in-memory size after refresh, not the raw registry file', async () => {
|
||||
(listRegisteredRepos as any).mockImplementation(async (opts?: { validate?: boolean }) =>
|
||||
opts?.validate
|
||||
? [MOCK_REPO_ENTRY]
|
||||
: [
|
||||
MOCK_REPO_ENTRY,
|
||||
{
|
||||
...MOCK_REPO_ENTRY,
|
||||
name: 'ghost-project',
|
||||
path: '/tmp/ghost-project',
|
||||
storagePath: '/tmp/.gitnexus/ghost-project',
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
expect(backend.cachedRepoCount()).toBe(0);
|
||||
await backend.init();
|
||||
expect(backend.cachedRepoCount()).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('LocalBackend.disconnect', () => {
|
||||
let backend: LocalBackend;
|
||||
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ function createMockBackend() {
|
|||
return {
|
||||
callTool: vi.fn().mockResolvedValue({ result: 'ok' }),
|
||||
listRepos: vi.fn().mockResolvedValue([]),
|
||||
countRepos: vi.fn().mockResolvedValue(0),
|
||||
resolveRepo: vi
|
||||
.fn()
|
||||
.mockResolvedValue({ name: 'test', repoPath: '/tmp/test', lastCommit: 'abc' }),
|
||||
|
|
|
|||
|
|
@ -37,6 +37,8 @@ const REPOS: RepoListing[] = [
|
|||
function createBackend(repos = REPOS) {
|
||||
return {
|
||||
listRepos: vi.fn().mockResolvedValue(repos.map((repo) => ({ ...repo }))),
|
||||
countRepos: vi.fn().mockResolvedValue(repos.length),
|
||||
cachedRepoCount: vi.fn().mockReturnValue(repos.length),
|
||||
callTool: vi.fn().mockImplementation(async (name: string, args: Record<string, unknown>) => ({
|
||||
name,
|
||||
args,
|
||||
|
|
@ -143,6 +145,21 @@ describe('MCP repository policy', () => {
|
|||
expect(backend.callTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('aligns unrestricted schema with the refreshed snapshot when count and cache diverge', async () => {
|
||||
const backend = createBackend();
|
||||
vi.mocked(backend.countRepos).mockResolvedValue(2);
|
||||
vi.mocked(backend.cachedRepoCount).mockReturnValue(1);
|
||||
const policy = await createMcpRepositoryPolicy(backend, {});
|
||||
await expect(policy.toolSchemaRepoRequirements(backend)).resolves.toEqual({
|
||||
readOnlyRequiresRepo: false,
|
||||
mutatingRequiresRepo: false,
|
||||
});
|
||||
expect(backend.selectToolRepository).toHaveBeenCalledWith(undefined, undefined, {
|
||||
allowCwdDefault: true,
|
||||
refreshRegistry: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps restricted schemas explicit when a multi-repo allowlist listing shrinks', async () => {
|
||||
const backend = createBackend();
|
||||
const policy = await createMcpRepositoryPolicy(backend, {
|
||||
|
|
|
|||
|
|
@ -31,6 +31,8 @@ function createMockBackend(overrides: Record<string, any> = {}): any {
|
|||
return {
|
||||
callTool: vi.fn().mockResolvedValue({ result: 'ok' }),
|
||||
listRepos: vi.fn().mockResolvedValue([]),
|
||||
countRepos: vi.fn().mockResolvedValue(0),
|
||||
cachedRepoCount: vi.fn().mockReturnValue(0),
|
||||
resolveRepo: vi
|
||||
.fn()
|
||||
.mockResolvedValue({ name: 'test', repoPath: '/tmp/test', lastCommit: 'abc' }),
|
||||
|
|
@ -110,6 +112,7 @@ describe('createMCPServer', () => {
|
|||
});
|
||||
it('requires repo in repo-scoped tool schemas when cwd cannot resolve multiple repos', async () => {
|
||||
const backend = createMockBackend({
|
||||
countRepos: vi.fn().mockResolvedValue(2),
|
||||
listRepos: vi.fn().mockResolvedValue([
|
||||
{ name: 'alpha', path: '/tmp/alpha' },
|
||||
{ name: 'beta', path: '/tmp/beta' },
|
||||
|
|
@ -139,6 +142,8 @@ describe('createMCPServer', () => {
|
|||
|
||||
it('keeps repo optional when cwd resolves one of multiple visible repos', async () => {
|
||||
const backend = createMockBackend({
|
||||
countRepos: vi.fn().mockResolvedValue(2),
|
||||
cachedRepoCount: vi.fn().mockReturnValue(2),
|
||||
listRepos: vi.fn().mockResolvedValue([
|
||||
{ name: 'alpha', path: '/tmp/alpha' },
|
||||
{ name: 'beta', path: '/tmp/beta' },
|
||||
|
|
@ -162,11 +167,12 @@ describe('createMCPServer', () => {
|
|||
const response = await client.callTool({ name: 'context', arguments: { name: 'Example' } });
|
||||
expect(response.isError).not.toBe(true);
|
||||
expect(backend.callTool).toHaveBeenCalledWith('context', { name: 'Example' });
|
||||
expect(backend.listRepos).toHaveBeenCalledTimes(1);
|
||||
expect(backend.countRepos).toHaveBeenCalledTimes(1);
|
||||
expect(backend.listRepos).not.toHaveBeenCalled();
|
||||
expect(backend.selectToolRepository).toHaveBeenCalledTimes(1);
|
||||
expect(backend.selectToolRepository).toHaveBeenCalledWith(undefined, undefined, {
|
||||
allowCwdDefault: true,
|
||||
refreshRegistry: false,
|
||||
refreshRegistry: true,
|
||||
});
|
||||
} finally {
|
||||
await client.close();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue