chore(hygiene): batch 5 LOW-severity follow-ups from RAN-6 review (RAN-33) (#70)

Bundles five low-severity hygiene items so reviewers don't pay per-PR
overhead on items that aren't individually severe.

1. Determinism — Analyzer breakdown-map ordering
   `nodeBreakdown`, `edgeBreakdown`, `frameworkBreakdown`, and
   `languageBreakdown` were `HashMap`s, which made the JSON serialization
   order of `AnalysisResult` non-deterministic across runs. Switched the
   12 declarations across `run`, `runBatchedIndex`, and `runSmartIndex`
   to `TreeMap`. Adds `breakdownMapsAreSortedDeterministically` test
   covering all four breakdowns.

2. Branding hygiene — VersionCommand + BundleCommand
   - VersionCommand banner: stale `Code IQ` → `codeiq` (matches the
     post-rename CLI / package / repo branding).
   - BundleCommand: added a footnote on the bundle-structure javadoc
     explaining that `code-iq-*-cli.jar` keeps the historical filename
     because it tracks the Maven `artifactId` (intentionally unchanged
     per `CLAUDE.md`). Future readers won't trip over it.
   Other remaining `code-iq` strings (the `<artifactId>` snippet in the
   README and the JAR/Maven-URL templates inside BundleCommand) are
   intentional Maven-coordinate references and stay as-is.

3. CORS defaults explicit — CorsConfig
   Promoted defaults to named constants
   (`DEFAULT_ALLOWED_ORIGIN_PATTERNS`, `API_ALLOWED_METHODS`,
   `MCP_ALLOWED_METHODS`, `ALLOWED_HEADERS`) and added a javadoc
   explaining the read-only API posture (no `PUT`/`PATCH`/`DELETE`)
   and the MCP `GET`/`POST` requirement. Behaviour unchanged — existing
   `CorsConfigTest` still passes verbatim. The `@Value` default still
   inlines the constant, and the field initializer keeps direct
   `new CorsConfig()` test usage working.

4. ExecutorService close/lifecycle regression coverage
   Promoted `Analyzer.BoundedExecutor` from `private` to package-private
   so its bounded-shutdown contract can be tested without spinning the
   full pipeline. Added `AnalyzerBoundedExecutorTest` with three cases:
     - long-running task: `close()` returns inside the
       graceful (10s) + force (5s) window and the task is interrupted;
     - idle executor: `close()` returns promptly;
     - interrupted closer thread: delegate is still shut down and the
       caller's interrupt flag is restored.

5. IntelligenceController.evidence — symlink hardening
   The `/api/intelligence/evidence` endpoint had only the lexical
   `normalize` + `startsWith` guard. Aligned it with the two-stage
   `toRealPath` guard that already protects `/api/file` (RAN-8): after
   the lexical check, resolve symlinks via `Path.toRealPath()` and
   re-check containment against the canonical root.
   `NoSuchFileException` is treated as "logical-only graph reference"
   and the lexical guard is sufficient (no symlink to traverse).
   Added `evidenceEndpointRejectsSymlinkEscapingRoot` and
   `evidenceEndpointAllowsInRepoSymlink`, both skip gracefully on
   filesystems without symlink support — same shape as the existing
   GraphController symlink tests.

Verification
- `mvn test -Dnpm.skip=true ...` → 3401 tests run, 0 failures, 0 errors,
  31 skipped (E2E without external repos).
- Targeted reruns of `AnalyzerTest`, `AnalyzerBoundedExecutorTest`,
  `CorsConfigTest`, `IntelligenceControllerTest`, `VersionCommandTest`
  all green.

Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 3 people

src/main/java/io/github/randomcodespace/iq/analyzer/Analyzer.java

@@ -275,7 +275,7 @@ private AnalysisResult runWithCache(Path root, Integer parallelism, AnalysisCach
         RepositoryIdentity repoIdentity = RepositoryIdentity.resolve(root);
 
         // Compute language breakdown
-        Map<String, Integer> languageBreakdown = new HashMap<>();
+        Map<String, Integer> languageBreakdown = new TreeMap<>();
         for (DiscoveredFile f : files) {
             languageBreakdown.merge(f.language(), 1, Integer::sum);
         }
@@ -425,21 +425,21 @@ private AnalysisResult runWithCache(Path root, Integer parallelism, AnalysisCach
         }
 
         // 7. Compute node breakdown
-        Map<String, Integer> nodeBreakdown = new HashMap<>();
+        Map<String, Integer> nodeBreakdown = new TreeMap<>();
         for (CodeNode node : allNodes) {
             String kindValue = node.getKind().getValue();
             nodeBreakdown.merge(kindValue, 1, Integer::sum);
         }
 
         // 8. Compute edge breakdown
-        Map<String, Integer> edgeBreakdown = new HashMap<>();
+        Map<String, Integer> edgeBreakdown = new TreeMap<>();
         for (var edge : builder.getEdges()) {
             String kindValue = edge.getKind().getValue();
             edgeBreakdown.merge(kindValue, 1, Integer::sum);
         }
 
         // 7b. Compute framework breakdown from node properties
-        Map<String, Integer> frameworkBreakdown = new HashMap<>();
+        Map<String, Integer> frameworkBreakdown = new TreeMap<>();
         for (CodeNode node : allNodes) {
             Object fw = node.getProperties().get(PROP_FRAMEWORK);
             if (fw != null && !fw.toString().isEmpty()) {
@@ -561,7 +561,7 @@ private AnalysisResult runBatchedWithCache(Path root, Integer parallelism, int b
         report.accept("Found " + totalFiles + " files");
 
         // Compute language breakdown
-        Map<String, Integer> languageBreakdown = new HashMap<>();
+        Map<String, Integer> languageBreakdown = new TreeMap<>();
         for (DiscoveredFile f : files) {
             languageBreakdown.merge(f.language(), 1, Integer::sum);
         }
@@ -575,9 +575,9 @@ private AnalysisResult runBatchedWithCache(Path root, Integer parallelism, int b
         int filesAnalyzed = 0;
         int cacheHits = 0;
         int batchNumber = 0;
-        Map<String, Integer> nodeBreakdown = new HashMap<>();
-        Map<String, Integer> edgeBreakdown = new HashMap<>();
-        Map<String, Integer> frameworkBreakdown = new HashMap<>();
+        Map<String, Integer> nodeBreakdown = new TreeMap<>();
+        Map<String, Integer> edgeBreakdown = new TreeMap<>();
+        Map<String, Integer> frameworkBreakdown = new TreeMap<>();
 
         // Clear previous index data if not incremental
         if (!incremental) {
@@ -852,7 +852,7 @@ private AnalysisResult runSmartWithCache(Path root, Integer parallelism, int bat
         int totalFiles = allFiles.size();
 
         // Compute language breakdown
-        Map<String, Integer> languageBreakdown = new HashMap<>();
+        Map<String, Integer> languageBreakdown = new TreeMap<>();
         for (DiscoveredFile f : allFiles) {
             languageBreakdown.merge(f.language(), 1, Integer::sum);
         }
@@ -875,9 +875,9 @@ private AnalysisResult runSmartWithCache(Path root, Integer parallelism, int bat
         int filesSkipped = 0;
         int cacheHits = 0;
         int batchNumber = 0;
-        Map<String, Integer> nodeBreakdown = new HashMap<>();
-        Map<String, Integer> edgeBreakdown = new HashMap<>();
-        Map<String, Integer> frameworkBreakdown = new HashMap<>();
+        Map<String, Integer> nodeBreakdown = new TreeMap<>();
+        Map<String, Integer> edgeBreakdown = new TreeMap<>();
+        Map<String, Integer> frameworkBreakdown = new TreeMap<>();
 
         // Process modules in sorted order for determinism
         List<String> sortedModuleKeys = new ArrayList<>(modules.keySet());
@@ -1354,8 +1354,11 @@ DetectorResult analyzeFileWithRegistry(DiscoveredFile file, Path repoPath,
      * Wrapper around ExecutorService that implements AutoCloseable with a bounded
      * shutdown — prevents the default close() from hanging up to 24 hours on stuck
      * ANTLR threads.
+     *
+     * <p>Package-private so the close/lifecycle behaviour can be regression-tested
+     * directly without spinning the full Analyzer pipeline.
      */
-    private record BoundedExecutor(java.util.concurrent.ExecutorService delegate) implements AutoCloseable {
+    record BoundedExecutor(java.util.concurrent.ExecutorService delegate) implements AutoCloseable {
         <T> Future<T> submit(java.util.concurrent.Callable<T> task) { return delegate.submit(task); }
 
         @Override

src/main/java/io/github/randomcodespace/iq/api/IntelligenceController.java

@@ -15,6 +15,9 @@
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.server.ResponseStatusException;
 
+import java.io.IOException;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
 import java.util.Map;
 
 /**
@@ -45,7 +48,10 @@ public IntelligenceController(
      * Assemble an evidence pack for a symbol or file path.
      *
      * <p>At least one of {@code symbol} or {@code file} must be provided.
-     * The {@code file} parameter is path-traversal guarded.
+     * The {@code file} parameter is path-traversal guarded with the same two-stage
+     * (lexical {@code normalize} then {@link Path#toRealPath} re-check) guard used
+     * by {@code GraphController.readFile} and the MCP {@code read_file} tool, so a
+     * symlink inside the indexed repo cannot be used to leak off-tree files.
      *
      * @param symbol          symbol name to look up
      * @param file            file path relative to repo root (path traversal guarded)
@@ -68,15 +74,35 @@ public EvidencePack getEvidence(
                     "At least one of 'symbol' or 'file' must be provided.");
         }
 
-        // Path traversal guard on file param
+        // Path-traversal guard on file param: two-stage lexical + symlink check.
         if (file != null && !file.isBlank()) {
-            java.nio.file.Path root = java.nio.file.Path.of(config.getRootPath())
-                    .toAbsolutePath().normalize();
-            java.nio.file.Path resolved = root.resolve(file).normalize();
-            if (!resolved.startsWith(root)) {
+            Path root;
+            try {
+                root = Path.of(config.getRootPath()).toRealPath();
+            } catch (IOException e) {
+                throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR,
+                        "Failed to resolve codebase root: " + e.getMessage());
+            }
+            Path candidate = root.resolve(file).normalize();
+            if (!candidate.startsWith(root)) {
                 throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
                         "Invalid file path: path traversal detected.");
             }
+            // Resolve symlinks if the file exists on disk and re-check containment.
+            // If the file is logical-only (graph reference, no on-disk file), the
+            // lexical guard above is sufficient — there is no symlink to traverse.
+            try {
+                Path real = candidate.toRealPath();
+                if (!real.startsWith(root)) {
+                    throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+                            "Invalid file path: path traversal detected.");
+                }
+            } catch (NoSuchFileException ignored) {
+                // file may exist only as a graph reference — lexical guard already passed
+            } catch (IOException e) {
+                throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR,
+                        "Failed to resolve file path: " + e.getMessage());
+            }
         }
 
         EvidencePackRequest request = new EvidencePackRequest(symbol, file, maxSnippetLines, includeRefs);

src/main/java/io/github/randomcodespace/iq/cli/BundleCommand.java

@@ -46,6 +46,10 @@
  * ├── flow.html          (interactive architecture diagram)
  * └── code-iq-*-cli.jar  (optional)
  * </pre>
+ * <p>
+ * The bundled CLI JAR keeps the historical {@code code-iq-*-cli.jar} filename because
+ * it tracks the Maven artifactId ({@code io.github.randomcodespace.iq:code-iq}), which
+ * is intentionally unchanged across the codeiq rename. See {@code CLAUDE.md}.
  */
 @Component
 @Command(name = "bundle", mixinStandardHelpOptions = true,

src/main/java/io/github/randomcodespace/iq/cli/VersionCommand.java

@@ -58,7 +58,7 @@ public Integer call() {
             allLanguages.addAll(d.getSupportedLanguages());
         }
 
-        CliOutput.bold("Code IQ " + version);
+        CliOutput.bold("codeiq " + version);
         CliOutput.info("  Java:       " + System.getProperty("java.version")
                 + " (" + System.getProperty("java.vendor") + ")");
         CliOutput.info("  Runtime:    " + System.getProperty("java.runtime.name"));

src/main/java/io/github/randomcodespace/iq/config/CorsConfig.java

@@ -7,12 +7,38 @@
 import org.springframework.web.servlet.config.annotation.CorsRegistry;
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
 
+/**
+ * CORS configuration for the {@code serving} profile.
+ *
+ * <p>The serving layer is strictly read-only: clients only need {@code GET} on the REST API
+ * and {@code GET}/{@code POST} on the MCP streamable-HTTP endpoint. Mutating verbs
+ * ({@code PUT}, {@code PATCH}, {@code DELETE}) are intentionally not allowed —
+ * analysis happens locally via the CLI ({@code codeiq index} / {@code codeiq enrich})
+ * and the server never accepts data manipulation.
+ *
+ * <p>Default origin patterns cover the common local-dev cases (loopback on any port).
+ * Override via {@code codeiq.cors.allowed-origin-patterns} (CSV) when serving over a
+ * trusted network or behind a reverse proxy.
+ */
 @Configuration
 @Profile("serving")
 public class CorsConfig {
 
-    @Value("${codeiq.cors.allowed-origin-patterns:http://localhost:[*],http://127.0.0.1:[*]}")
-    private String allowedOriginPatterns = "http://localhost:[*],http://127.0.0.1:[*]";
+    /** Default allowed origin patterns: loopback on any port (covers local dev / IDE proxies). */
+    static final String DEFAULT_ALLOWED_ORIGIN_PATTERNS =
+            "http://localhost:[*],http://127.0.0.1:[*]";
+
+    /** Read-only REST API: only safe / preflight verbs. */
+    static final String[] API_ALLOWED_METHODS = {"GET", "OPTIONS"};
+
+    /** MCP streamable-HTTP: GET for SSE/handshake, POST for JSON-RPC frames, OPTIONS for preflight. */
+    static final String[] MCP_ALLOWED_METHODS = {"GET", "POST", "OPTIONS"};
+
+    /** Allow all request headers — clients commonly send custom MCP / Auth headers. */
+    static final String ALLOWED_HEADERS = "*";
+
+    @Value("${codeiq.cors.allowed-origin-patterns:" + DEFAULT_ALLOWED_ORIGIN_PATTERNS + "}")
+    private String allowedOriginPatterns = DEFAULT_ALLOWED_ORIGIN_PATTERNS;
 
     @Bean
     public WebMvcConfigurer corsConfigurer() {
@@ -22,12 +48,12 @@ public WebMvcConfigurer corsConfigurer() {
             public void addCorsMappings(CorsRegistry registry) {
                 registry.addMapping("/api/**")
                         .allowedOriginPatterns(patterns)
-                        .allowedMethods("GET", "OPTIONS")
-                        .allowedHeaders("*");
+                        .allowedMethods(API_ALLOWED_METHODS)
+                        .allowedHeaders(ALLOWED_HEADERS);
                 registry.addMapping("/mcp/**")
                         .allowedOriginPatterns(patterns)
-                        .allowedMethods("GET", "POST", "OPTIONS")
-                        .allowedHeaders("*");
+                        .allowedMethods(MCP_ALLOWED_METHODS)
+                        .allowedHeaders(ALLOWED_HEADERS);
             }
         };
     }

src/test/java/io/github/randomcodespace/iq/analyzer/AnalyzerBoundedExecutorTest.java

@@ -0,0 +1,113 @@
+package io.github.randomcodespace.iq.analyzer;
+
+import org.junit.jupiter.api.Test;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Regression coverage for {@link Analyzer.BoundedExecutor}: the default
+ * {@code ExecutorService.close()} can block up to 24 hours waiting for stuck
+ * ANTLR parser threads. The wrapper enforces a graceful 10s shutdown followed
+ * by a 5s {@code shutdownNow} window.
+ */
+class AnalyzerBoundedExecutorTest {
+
+    /** Hard upper bound on close() = graceful 10s + force 5s + scheduling slack. */
+    private static final long MAX_CLOSE_SECONDS = 18;
+
+    @Test
+    void close_completes_within_bounded_window_for_long_running_task() throws Exception {
+        ExecutorService delegate = Executors.newSingleThreadExecutor();
+        Analyzer.BoundedExecutor executor = new Analyzer.BoundedExecutor(delegate);
+
+        AtomicBoolean wasInterrupted = new AtomicBoolean();
+        CountDownLatch started = new CountDownLatch(1);
+        Future<Void> task = executor.submit(() -> {
+            started.countDown();
+            try {
+                // Far longer than the bounded close() window.
+                Thread.sleep(TimeUnit.MINUTES.toMillis(5));
+            } catch (InterruptedException e) {
+                wasInterrupted.set(true);
+                Thread.currentThread().interrupt();
+            }
+            return null;
+        });
+        assertNotNull(task);
+        assertTrue(started.await(5, TimeUnit.SECONDS), "submitted task should start");
+
+        Instant t0 = Instant.now();
+        executor.close();
+        Duration elapsed = Duration.between(t0, Instant.now());
+
+        assertTrue(delegate.isShutdown(),
+                "delegate must be shutdown after close()");
+        assertTrue(elapsed.toSeconds() < MAX_CLOSE_SECONDS,
+                "close() must respect bounded shutdown window (max "
+                        + MAX_CLOSE_SECONDS + "s), got " + elapsed);
+        // shutdownNow should have interrupted the sleeping task.
+        assertTrue(wasInterrupted.get(),
+                "blocked task must be interrupted by shutdownNow");
+    }
+
+    @Test
+    void close_is_immediate_when_executor_is_idle() {
+        ExecutorService delegate = Executors.newSingleThreadExecutor();
+        Analyzer.BoundedExecutor executor = new Analyzer.BoundedExecutor(delegate);
+
+        Instant t0 = Instant.now();
+        executor.close();
+        Duration elapsed = Duration.between(t0, Instant.now());
+
+        assertTrue(delegate.isShutdown());
+        assertTrue(delegate.isTerminated());
+        // Idle close should return well under the graceful window.
+        assertTrue(elapsed.toMillis() < 2_000,
+                "idle close() should return promptly, got " + elapsed);
+    }
+
+    @Test
+    void close_propagates_interrupt_to_caller_thread() throws Exception {
+        ExecutorService delegate = Executors.newSingleThreadExecutor();
+        Analyzer.BoundedExecutor executor = new Analyzer.BoundedExecutor(delegate);
+
+        CountDownLatch started = new CountDownLatch(1);
+        executor.submit(() -> {
+            started.countDown();
+            try {
+                Thread.sleep(TimeUnit.MINUTES.toMillis(5));
+            } catch (InterruptedException ignored) {
+                // swallow — we're testing the wrapper, not the task
+            }
+            return null;
+        });
+        assertTrue(started.await(5, TimeUnit.SECONDS));
+
+        AtomicBoolean closerInterrupted = new AtomicBoolean();
+        Thread closer = new Thread(() -> {
+            executor.close();
+            closerInterrupted.set(Thread.currentThread().isInterrupted());
+        });
+        closer.start();
+        // Let the closer enter awaitTermination, then interrupt it.
+        Thread.sleep(200);
+        closer.interrupt();
+        closer.join(TimeUnit.SECONDS.toMillis(MAX_CLOSE_SECONDS));
+
+        assertFalse(closer.isAlive(), "closer thread should finish after interrupt");
+        assertTrue(delegate.isShutdown(), "interrupt path must still shutdown the delegate");
+        assertTrue(closerInterrupted.get(),
+                "wrapper must restore the caller's interrupt flag");
+    }
+}

src/test/java/io/github/randomcodespace/iq/analyzer/AnalyzerTest.java

@@ -85,6 +85,44 @@ void analyzesJavaFiles() throws IOException {
         assertTrue(result.elapsed().toMillis() >= 0);
     }
 
+    /**
+     * Regression: AnalysisResult breakdown maps must iterate in deterministic
+     * sorted order so that JSON serialization is byte-stable across runs.
+     */
+    @Test
+    void breakdownMapsAreSortedDeterministically() throws IOException {
+        // File names chosen so SERVICE / CLASS / and the kind values would not
+        // appear in sorted order under the previous HashMap implementation.
+        Files.writeString(tempDir.resolve("Zeta.java"), "public class Zeta {}");
+        Files.writeString(tempDir.resolve("Alpha.java"), "public class Alpha {}");
+        Files.writeString(tempDir.resolve("Mu.java"), "public class Mu {}");
+
+        AnalysisResult result = analyzer.run(tempDir, progressMessages::add);
+
+        assertSortedKeys(result.languageBreakdown().keySet().stream().toList(),
+                "languageBreakdown");
+        assertSortedKeys(result.nodeBreakdown().keySet().stream().toList(),
+                "nodeBreakdown");
+        assertSortedKeys(result.edgeBreakdown().keySet().stream().toList(),
+                "edgeBreakdown");
+        assertSortedKeys(result.frameworkBreakdown().keySet().stream().toList(),
+                "frameworkBreakdown");
+
+        // Sanity: at least the kinds we expect should be present.
+        assertTrue(result.nodeBreakdown().keySet().contains("class"));
+        assertTrue(result.nodeBreakdown().keySet().contains("service"));
+    }
+
+    private static void assertSortedKeys(List<String> keys, String name) {
+        for (int i = 1; i < keys.size(); i++) {
+            String prev = keys.get(i - 1);
+            String cur = keys.get(i);
+            assertTrue(prev.compareTo(cur) < 0,
+                    name + " not in sorted order at index " + i + ": '"
+                            + prev + "' >= '" + cur + "' (full: " + keys + ")");
+        }
+    }
+
     @Test
     void reportsProgress() throws IOException {
         Files.writeString(tempDir.resolve("App.java"), "public class App {}");