Merge branch 'main' into docs/kilo-code-mcp

gitnexus/src/core/group/extractors/http-patterns/java.ts

@@ -678,13 +678,16 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = {
   language: Java,
   // routeCoverage intentionally LEFT at the default 'partial' (#2138 Part 2).
   // The graph provider set is a strict *subset* of this scan()'s provider set —
-  // ingestion does NOT emit a Route node for (1) array-form `@GetMapping({...})`,
-  // (2) interface-inherited Spring routes, or (3) the 2nd verb of a same-URL
-  // GET+POST pair (Route nodes are URL-keyed). Declaring 'complete' here would
-  // let the parse-skip drop those group-only providers. Java flips to 'complete'
-  // only once ingestion provider extraction matches this scan (a follow-up:
-  // array-form query branch + interface-inheritance emission + per-verb Route
-  // identity). `hasConsumerSignals` below is kept ready for that flip.
+  // ingestion does NOT emit a Route node for (1) a method-level array route
+  // nested under a class-level array-form `@RequestMapping` (ingestion suppresses
+  // it rather than drop the prefix; bare/scalar-prefixed array methods ARE now
+  // emitted — see #2280), (2) interface-inherited Spring routes, or (3) the 2nd
+  // verb of a same-URL GET+POST pair (Route nodes are URL-keyed). Declaring
+  // 'complete' here would let the parse-skip drop those group-only providers.
+  // Java flips to 'complete' only once ingestion provider extraction matches this
+  // scan (a follow-up: class-level array-form prefix support + interface-
+  // inheritance emission + per-verb Route identity — tracked in #2280).
+  // `hasConsumerSignals` below is kept ready for that flip.
   // Consumer signals this plugin's scan() can detect: RestTemplate / WebClient /
   // OkHttp / Java-HttpClient / Apache-HttpClient call sites, OpenFeign
   // (`@FeignClient` + `@RequestLine`) interfaces, and Spring 6 HTTP Interface

gitnexus/src/core/ingestion/route-extractors/spring.ts

@@ -39,6 +39,18 @@ import {
  *   @node  → enclosing declaration (class_declaration | method_declaration)
  *   @value → the string-literal argument
  *   @key   → the named-argument member key (absent for positional form)
+ *
+ * Method-level routes accept both the bare string form `@GetMapping("/x")` and
+ * the array form `@GetMapping({"/a","/b"})` (positional or `path =`/`value =`):
+ * a multi-element array yields one match per element, so the Phase 2 loop emits
+ * one route per path with no special-casing. This mirrors the group-layer
+ * `java.ts` query so the two Spring extractors stay in parity (#2138 follow-up;
+ * the divergence here was the root of the #2265 array-form gap). The class-level
+ * `@RequestMapping` branches also match the array form, but only to *detect* it:
+ * an array-form class prefix can't be resolved to a single string, so Phase 2
+ * suppresses that class's method-level array routes rather than emit them with a
+ * dropped prefix (a wrong route). Full class-array cross-product support is left
+ * to a follow-up (#2280).
  */
 const ROUTE_ANNOTATION_QUERY = new Parser.Query(
   Java,
@@ -48,7 +60,9 @@ const ROUTE_ANNOTATION_QUERY = new Parser.Query(
       (modifiers
         (annotation
           name: (identifier) @ann
-          arguments: (annotation_argument_list (string_literal) @value)))) @node
+          arguments: (annotation_argument_list
+            [(string_literal) @value
+             (element_value_array_initializer (string_literal) @value)])))) @node
     (class_declaration
       (modifiers
         (annotation
@@ -56,12 +70,15 @@ const ROUTE_ANNOTATION_QUERY = new Parser.Query(
           arguments: (annotation_argument_list
             (element_value_pair
               key: (identifier) @key
-              value: (string_literal) @value))))) @node
+              value: [(string_literal) @value
+                      (element_value_array_initializer (string_literal) @value)]))))) @node
     (method_declaration
       (modifiers
         (annotation
           name: (identifier) @ann
-          arguments: (annotation_argument_list (string_literal) @value)))) @node
+          arguments: (annotation_argument_list
+            [(string_literal) @value
+             (element_value_array_initializer (string_literal) @value)])))) @node
     (method_declaration
       (modifiers
         (annotation
@@ -69,7 +86,8 @@ const ROUTE_ANNOTATION_QUERY = new Parser.Query(
           arguments: (annotation_argument_list
             (element_value_pair
               key: (identifier) @key
-              value: (string_literal) @value))))) @node
+              value: [(string_literal) @value
+                      (element_value_array_initializer (string_literal) @value)]))))) @node
   ]
 `,
 );
@@ -93,8 +111,15 @@ export function extractSpringRoutes(
 ): ExtractedDecoratorRoute[] {
   const matches = ROUTE_ANNOTATION_QUERY.matches(tree.rootNode);
 
-  // Phase 1: collect class-level @RequestMapping prefixes keyed by node id
+  // Phase 1: collect class-level @RequestMapping prefixes keyed by node id.
+  // A scalar prefix (`@RequestMapping("/base")`) is stored in prefixByClassId.
+  // A class whose @RequestMapping uses the array form (`@RequestMapping({...})`)
+  // is instead recorded in classesWithArrayPrefix: there is no single prefix to
+  // store, and Phase 2 uses this to suppress that class's method-level array
+  // routes rather than emit them unprefixed (a wrong route — see #2280). Full
+  // class-array cross-product support is out of scope here.
   const prefixByClassId = new Map<number, string>();
+  const classesWithArrayPrefix = new Set<number>();
 
   for (const match of matches) {
     const caps: Record<string, Parser.SyntaxNode> = {};
@@ -109,6 +134,10 @@ export function extractSpringRoutes(
 
     if (node.type === 'class_declaration' && annNode.text === 'RequestMapping') {
       if (!isRouteMemberKey(keyNode)) continue;
+      if (valueNode.parent?.type === 'element_value_array_initializer') {
+        classesWithArrayPrefix.add(node.id);
+        continue;
+      }
       const prefix = unquoteSpringLiteral(valueNode.text);
       if (prefix !== null) prefixByClassId.set(node.id, prefix);
     }
@@ -138,6 +167,20 @@ export function extractSpringRoutes(
     const routePath = unquoteSpringLiteral(valueNode.text);
     if (routePath === null) continue;
     const enclosingClass = findEnclosingClass(node);
+
+    // Suppress a method-level *array-form* route nested under a class-level
+    // array-form @RequestMapping. The class prefix is one of several values that
+    // cannot be resolved to a single string here, so emitting the route would
+    // drop the prefix and yield a wrong unprefixed Route (a false signal, worse
+    // than a missing one). Skipping keeps ingestion a strict subset of the group
+    // scan — safe under routeCoverage:'partial'. Full class-array cross-product
+    // support is tracked in #2280. (Scalar method paths under an array class
+    // prefix are left unchanged: that pre-existing divergence is out of scope.)
+    const isArrayElement = valueNode.parent?.type === 'element_value_array_initializer';
+    if (isArrayElement && enclosingClass && classesWithArrayPrefix.has(enclosingClass.id)) {
+      continue;
+    }
+
     const classPrefix = enclosingClass ? (prefixByClassId.get(enclosingClass.id) ?? '') : '';
     // `node` is the annotated `method_declaration`; its name field is the
     // handler method name (resolved to a symbol UID later by the routes phase).

gitnexus/test/integration/route-parse-skip.test.ts

@@ -173,7 +173,11 @@ public class AController {
 `;
     const dir = mkRepo({ 'AController.java': AC });
     try {
-      // Graph resolves only /covered (ingestion has no array-form Route node).
+      // The mock DB resolves only /covered; it deliberately omits the array-form
+      // routes to exercise the source-scan fallback. (Ingestion now DOES emit
+      // array-form Route nodes under a scalar/absent class prefix — see #2280 —
+      // but this test asserts the group extractor still recovers them via scan
+      // when the graph happens to lack them, which is what 'partial' guarantees.)
       const out = await new HttpRouteExtractor().extract(
         makeDb([
           { file: 'AController.java', routePath: '/covered', method: 'GET', resolved: true },
@@ -182,7 +186,7 @@ public class AController {
         repo,
       );
       const paths = providerPaths(out);
-      // The array-form routes are graph-only-absent but survive via source scan.
+      // The array-form routes are absent from this mock graph but survive via source scan.
       expect(paths).toEqual(expect.arrayContaining(['GET::/a', 'GET::/b']));
     } finally {
       fs.rmSync(dir, { recursive: true, force: true });

gitnexus/test/unit/group/spring-route-parity.test.ts

@@ -0,0 +1,199 @@
+/**
+ * Parity test for the two Spring route extractors (#2078 maintainer request,
+ * #2138 follow-up).
+ *
+ * GitNexus parses Spring `@(Get|Post|...)Mapping` annotations in TWO places:
+ *   - ingestion `route-extractors/spring.ts` → `extractSpringRoutes` (produces
+ *     graph `Route` nodes)
+ *   - group `http-patterns/java.ts` → `JAVA_HTTP_PLUGIN.scan` (produces
+ *     cross-repo HTTP contracts)
+ *
+ * They serve different layers and stay separate, but they MUST agree on the
+ * set of provider (method, path) routes they recognise for the same source —
+ * otherwise the graph under-covers what the group scan sees, which is exactly
+ * the divergence behind the #2265 array-form gap (the group query matched
+ * `@GetMapping({"/a","/b"})`, ingestion's didn't). This test runs one shared
+ * fixture through both and asserts the provider sets are identical for the
+ * shapes ingestion claims to cover: bare, named-arg, and array-form method
+ * routes under a *scalar* (or absent) class prefix.
+ *
+ * Known, deliberate divergence (NOT covered by the equality assertions): a
+ * method-level array route nested under a class-level *array-form*
+ * @RequestMapping. Ingestion suppresses it (it can't resolve which of several
+ * class prefixes to apply, and a dropped-prefix route is a wrong signal), while
+ * the group layer emits the full cross-product. The last test pins this so the
+ * suppression can't silently regress into emitting wrong routes; full
+ * class-array support is tracked in #2280.
+ */
+import { describe, it, expect } from 'vitest';
+import Parser from 'tree-sitter';
+import Java from 'tree-sitter-java';
+import { extractSpringRoutes } from '../../../src/core/ingestion/route-extractors/spring.js';
+import { JAVA_HTTP_PLUGIN } from '../../../src/core/group/extractors/http-patterns/java.js';
+
+function parse(src: string): Parser.Tree {
+  const p = new Parser();
+  p.setLanguage(Java);
+  return p.parse(src);
+}
+
+/** Canonical `METHOD /a/b` form so prefix-join / slash / case differences wash out. */
+function canon(method: string, ...segments: string[]): string {
+  const path = `/${segments.join('/').split('/').filter(Boolean).join('/')}`;
+  return `${method.toUpperCase()} ${path.toLowerCase()}`;
+}
+
+/** ingestion side: join the class prefix + method path the way the routes phase does. */
+function ingestionProviders(src: string): Set<string> {
+  return new Set(
+    extractSpringRoutes(parse(src), 'X.java').map((r) =>
+      canon(r.httpMethod, r.prefix ?? '', r.routePath),
+    ),
+  );
+}
+
+/** group side: provider detections (path already prefix-joined by the plugin). */
+function groupProviders(src: string): Set<string> {
+  return new Set(
+    JAVA_HTTP_PLUGIN.scan(parse(src))
+      .filter((d) => d.role === 'provider')
+      .map((d) => canon(d.method, d.path)),
+  );
+}
+
+describe('Spring route extractor parity — ingestion spring.ts vs group java.ts', () => {
+  it('agree on bare, named-arg, and array-form method routes under a class prefix', () => {
+    const src = `package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping("/api/orders")
+public class OrderController {
+  @GetMapping("/list") public Object list() { return null; }
+  @PostMapping(path = "/make") public Object make() { return null; }
+  @PutMapping(value = "/update") public Object update() { return null; }
+  @GetMapping({"/a", "/b"}) public Object multi() { return null; }
+}
+`;
+    const ingestion = ingestionProviders(src);
+    const group = groupProviders(src);
+
+    // The array form is the regression that motivated this: both must see all four.
+    expect(group).toEqual(
+      new Set([
+        'GET /api/orders/list',
+        'POST /api/orders/make',
+        'PUT /api/orders/update',
+        'GET /api/orders/a',
+        'GET /api/orders/b',
+      ]),
+    );
+    expect(ingestion).toEqual(group);
+  });
+
+  it('agree on a no-prefix controller with a positional array', () => {
+    const src = `package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class PlainController {
+  @GetMapping("/solo") public Object solo() { return null; }
+  @DeleteMapping({"/x", "/y", "/z"}) public Object many() { return null; }
+}
+`;
+    expect(ingestionProviders(src)).toEqual(groupProviders(src));
+  });
+
+  it('agree on named-arg ARRAY forms (path = {...} / value = {...})', () => {
+    // Guards the spring.ts named-array query branch specifically: positional
+    // arrays alone would not exercise it.
+    const src = `package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping("/api")
+public class NamedArrayController {
+  @PutMapping(value = {"/update", "/modify"}) public Object upd() { return null; }
+  @DeleteMapping(path = {"/x", "/y"}) public Object del() { return null; }
+}
+`;
+    const group = groupProviders(src);
+    expect(group).toEqual(
+      new Set(['PUT /api/update', 'PUT /api/modify', 'DELETE /api/x', 'DELETE /api/y']),
+    );
+    expect(ingestionProviders(src)).toEqual(group);
+  });
+
+  it('do not leak non-route arrays (consumes/produces) as routes — array analogue', () => {
+    // The scalar `produces` anti-regression already exists in the route tests;
+    // this is its array form. `consumes`/`produces` arrays must never surface as
+    // provider routes; only the path value does.
+    const src = `package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class ContentTypeController {
+  @GetMapping(value = "/v", consumes = {"application/json", "application/xml"}, produces = {"application/json"})
+  public Object v() { return null; }
+  @PostMapping(consumes = {"application/json"})
+  public Object noPath() { return null; }
+}
+`;
+    const ingestion = ingestionProviders(src);
+    const group = groupProviders(src);
+    // Only the explicit path leaks through; the consumes/produces arrays do not,
+    // and the path-less @PostMapping contributes nothing.
+    expect(group).toEqual(new Set(['GET /v']));
+    expect(ingestion).toEqual(group);
+
+    // Pure-consumes controller (no path anywhere) → EMPTY provider set on both
+    // sides: a consumes/produces array must never be misread as a route path.
+    const consumesOnly = `package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class ConsumesOnlyController {
+  @PostMapping(consumes = {"application/json", "application/xml"})
+  public Object a() { return null; }
+  @PutMapping(produces = {"application/json"})
+  public Object b() { return null; }
+}
+`;
+    expect(groupProviders(consumesOnly)).toEqual(new Set());
+    expect(ingestionProviders(consumesOnly)).toEqual(new Set());
+  });
+
+  it('pins the deliberate class-array divergence: ingestion suppresses, group emits cross-product (#2280)', () => {
+    // A method-level array under a class-level ARRAY-form @RequestMapping. There
+    // is no single class prefix to apply, so ingestion suppresses the route
+    // rather than emit it unprefixed (a wrong signal). The group layer emits the
+    // full cross-product. This is a KNOWN gap (#2280), pinned here so the
+    // suppression can't silently regress into emitting wrong unprefixed routes.
+    const src = `package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping({"/base/one", "/base/two"})
+public class MultiPrefixController {
+  @GetMapping({"/primary", "/alias"}) public Object x() { return null; }
+}
+`;
+    const ingestion = ingestionProviders(src);
+    const group = groupProviders(src);
+
+    // group: correct cross-product of the two class prefixes × two method paths.
+    expect(group).toEqual(
+      new Set([
+        'GET /base/one/primary',
+        'GET /base/two/primary',
+        'GET /base/one/alias',
+        'GET /base/two/alias',
+      ]),
+    );
+    // ingestion: suppressed — emits NO route for the array method (never a wrong
+    // unprefixed `GET /primary` / `GET /alias`).
+    expect(ingestion).toEqual(new Set());
+    // And the divergence is asymmetric-by-design: ingestion ⊊ group here.
+    expect(ingestion).not.toEqual(group);
+  });
+});