From cc761ce7cd3d8c7f2a5f48552cdcd08b41a2af94 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Tue, 23 Jun 2026 17:21:08 +0000 Subject: [PATCH] fix(mcp): tolerate adapter-materialized line:0 in impact callgraph mode (#2279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some MCP client/agent adapters serialize an omitted optional numeric field as `0` rather than dropping it, so callgraph `impact` calls arrive carrying a spurious `line: 0`. `line` is a PDG-only statement anchor and is meaningless on the callgraph path, so the backend rejected the call ("'line' is only supported with mode:'pdg'") and strict clients rejected it client-side against the advertised `minimum: 1`. Treat a literal `line: 0` as omitted in `_impactImpl` when mode !== 'pdg' and let the normal symbol→symbol BFS run. The coercion is deliberately narrow: only the literal 0, only on the callgraph path. A genuine positive `line` on callgraph still errors (real mode mistake), negative/ fractional values still error, and pdg mode is untouched — `line: 0` there is still rejected (there is no 1-based source line 0 to anchor on). Regression tests pin the full matrix: callgraph + line:0 runs the BFS and is byte-identical to omitting line; pdg + line:0 still errors; positive line on callgraph still errors. Co-Authored-By: Claude Opus 4.8 (1M context) --- gitnexus/src/mcp/local/local-backend.ts | 18 ++++++-- gitnexus/test/unit/calltool-dispatch.test.ts | 45 ++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 10733727d..8ea6880e7 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -4490,9 +4490,21 @@ export class LocalBackend { } const mode = modeResult.mode; + // #2279: some MCP client/agent adapters serialize an *omitted* optional + // numeric field as `0` rather than dropping it, so callgraph calls arrive + // carrying a spurious `line: 0`. `line` is meaningless on the callgraph path + // (the symbol→symbol BFS has no statement notion), so treat a literal `0` + // there as omitted and let the normal traversal run. The coercion is + // deliberately narrow — only the literal `0`, only when mode !== 'pdg': + // a genuine positive `line` on callgraph still errors (real mode mistake), + // negative/fractional values still error, and pdg mode is untouched (the + // normalization is an identity there, so `line: 0` is still rejected below — + // there is no 1-based source line `0` to anchor on). + const effectiveLine = mode !== 'pdg' && params.line === 0 ? undefined : params.line; + // `line` is a PDG-only statement anchor. Reject it on the callgraph path // rather than silently ignore (the symbol→symbol BFS has no statement notion). - if (params.line !== undefined && mode !== 'pdg') { + if (effectiveLine !== undefined && mode !== 'pdg') { return { error: `Parameter 'line' is only supported with mode:'pdg' (it anchors the dependence slice on a statement). Remove it or set mode:'pdg'.`, target: { name: params.target }, @@ -4503,8 +4515,8 @@ export class LocalBackend { } // A provided `line` must be a positive integer. if ( - params.line !== undefined && - (!Number.isInteger(params.line) || (params.line as number) < 1) + effectiveLine !== undefined && + (!Number.isInteger(effectiveLine) || (effectiveLine as number) < 1) ) { // Line param fails validation before target resolution → partial-but-typed // target on the pdg path (typed PdgImpactTarget, not an inline literal). diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index a9e47bdef..9fc72086f 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -1642,6 +1642,51 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { }, ); + // #2279: some MCP client/agent adapters serialize an *omitted* optional + // numeric field as `0`. On the callgraph path `line` is meaningless, so a + // literal `line: 0` must be tolerated as omitted (NOT the PDG-only error) and + // route to the normal BFS — distinct from a genuine positive `line` (above), + // which stays a hard error. + it.each([['callgraph'], [undefined]])( + 'mode:%j + adapter-materialized line:0 is treated as omitted and runs the BFS (#2279)', + async (mode) => { + resolveSingleTarget(); + const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS'); + const result = await backend.callTool('impact', { + target: 'main', + direction: 'upstream', + mode: mode as any, + line: 0, + }); + // No PDG-only error, no positive-integer error — line:0 is swallowed. + expect(result.error ?? '').not.toMatch(/'line' is only supported with mode:'pdg'/); + expect(result.error ?? '').not.toMatch(/'line' must be a positive integer/); + expect(result.target).toBeDefined(); + expect(bfsSpy).toHaveBeenCalledTimes(1); + }, + ); + + it("mode:'callgraph'/undefined + line:0 is byte-identical to omitting line (#2279)", async () => { + resolveSingleTarget(); + const omitted = await backend.callTool('impact', { target: 'main', direction: 'upstream' }); + const callgraphZero = await backend.callTool('impact', { + target: 'main', + direction: 'upstream', + mode: 'callgraph', + line: 0, + }); + const undefZero = await backend.callTool('impact', { + target: 'main', + direction: 'upstream', + mode: undefined, + line: 0, + }); + // The normalization must leave the callgraph result indistinguishable from a + // call that never carried `line` — the spurious 0 must not leak into output. + expect(callgraphZero).toEqual(omitted); + expect(undefZero).toEqual(omitted); + }); + it.each([[0], [-1], [1.5]])( "mode:'pdg' + non-positive-integer line %j → structured {error}, never routed to traversal", async (badLine) => {