From 4d43223cb680421d44982c8f88cc0a7fc7914d1a Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Thu, 23 Apr 2026 14:57:49 +0000 Subject: [PATCH 1/3] fix(detector): ModuleDepsDetector reaches settings.gradle branch The dispatch chain in detect() checked `.endsWith(".gradle")` before the settings-specific branch, so any `settings.gradle` / `settings.gradle.kts` path routed to detectGradle() and the specialised detectGradleSettings() helper was never reached. Gradle multi-module `include ':foo'` entries were silently lost. Reordered the dispatch so settings files are matched first, and added ModuleDepsDetectorTest to lock in the reachability contract (plus a regression guard that `build.gradle` still routes to detectGradle). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../detector/jvm/java/ModuleDepsDetector.java | 11 ++- .../jvm/java/ModuleDepsDetectorTest.java | 97 +++++++++++++++++-- 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/src/main/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetector.java b/src/main/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetector.java index 4e5f2f4b..9dc2d947 100644 --- a/src/main/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetector.java +++ b/src/main/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetector.java @@ -61,11 +61,16 @@ public DetectorResult detect(DetectorContext ctx) { String filePath = ctx.filePath(); if (filePath.endsWith("pom.xml")) { return detectMaven(ctx); - } else if (filePath.endsWith(".gradle") || filePath.endsWith(".gradle.kts")) { - return detectGradle(ctx); - } else if (filePath.endsWith("settings.gradle") || filePath.endsWith("settings.gradle.kts")) { + } + // Order matters: `settings.gradle[.kts]` must be matched before the generic + // `.gradle[.kts]` branch, otherwise Gradle multi-module settings files are + // misrouted to detectGradle() and never reach detectGradleSettings(). + if (filePath.endsWith("settings.gradle") || filePath.endsWith("settings.gradle.kts")) { return detectGradleSettings(ctx); } + if (filePath.endsWith(".gradle") || filePath.endsWith(".gradle.kts")) { + return detectGradle(ctx); + } return DetectorResult.empty(); } diff --git a/src/test/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetectorTest.java b/src/test/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetectorTest.java index 49b809aa..1820818e 100644 --- a/src/test/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetectorTest.java +++ b/src/test/java/io/github/randomcodespace/iq/detector/jvm/java/ModuleDepsDetectorTest.java @@ -3,6 +3,7 @@ import io.github.randomcodespace.iq.detector.DetectorContext; import io.github.randomcodespace.iq.detector.DetectorResult; import io.github.randomcodespace.iq.detector.DetectorTestUtils; +import io.github.randomcodespace.iq.model.CodeNode; import io.github.randomcodespace.iq.model.EdgeKind; import io.github.randomcodespace.iq.model.NodeKind; import org.junit.jupiter.api.Test; @@ -15,6 +16,10 @@ * *

Covers Maven (pom.xml), Gradle build scripts (.gradle / .gradle.kts), and * Gradle settings (settings.gradle / settings.gradle.kts) detection paths. + * + *

The settings.gradle dispatch was previously shadowed by the generic + * {@code .endsWith(".gradle")} branch; the tests below both exercise the fixed + * path and guard against a regression to that bug. */ class ModuleDepsDetectorTest { @@ -263,16 +268,87 @@ void gradleKts_withGroovyStyleStringDep_isDetected() { .hasSize(1); } + @Test + void buildGradle_routesToDetectGradle_notToSettings() { + // Regression guard: `include ':x'` tokens that happen to appear in a build.gradle + // file must NOT be interpreted as settings-style module includes — those only apply + // inside settings.gradle. This pairs with the settings.gradle dispatch tests below. + String build = """ + dependencies { + implementation project(':shared') + implementation 'com.acme:lib:1.0.0' + } + """; + DetectorContext ctx = DetectorTestUtils.contextFor("build.gradle", "gradle", build); + DetectorResult r = detector.detect(ctx); + + // detectGradle emits a module node for the current file plus DEPENDS_ON edges. + assertThat(r.nodes()).isNotEmpty(); + assertThat(r.edges()).isNotEmpty(); + } + // --------------------------------------------------------------- - // Gradle settings.gradle + // Gradle settings.gradle — the fixed dispatch branch // --------------------------------------------------------------- // - // The detector's dispatch chain checks `.endsWith(".gradle")` *before* - // `.endsWith("settings.gradle")`, so any filename ending in `.gradle` - // routes to detectGradle (not detectGradleSettings). The - // detectGradleSettings() branch is therefore unreachable via the public - // detect() API for these filenames — flagged as a follow-up bug. - // We intentionally omit direct tests for that unreachable branch. + // Prior to the dispatch-order fix, `.endsWith(".gradle")` matched + // settings.gradle first, so detectGradleSettings() was unreachable via the + // public detect() API. Tests below both exercise the now-reachable path + // and pin the contract so the bug cannot reappear. + + @Test + void settingsGradle_routesToDetectGradleSettings_andEmitsModuleNodes() { + // `include 'foo'` is only handled by detectGradleSettings. + // detectGradle would emit zero module nodes for this input, so seeing + // module nodes here proves the dispatch fix reaches detectGradleSettings. + String settings = """ + rootProject.name = 'acme' + include ':api' + include ':domain' + include ':infra' + """; + DetectorContext ctx = DetectorTestUtils.contextFor("settings.gradle", "gradle", settings); + DetectorResult r = detector.detect(ctx); + + assertThat(r.nodes()) + .extracting(CodeNode::getLabel) + .containsExactlyInAnyOrder("api", "domain", "infra"); + assertThat(r.nodes()) + .allMatch(n -> n.getKind() == NodeKind.MODULE); + assertThat(r.nodes()) + .allMatch(n -> "gradle".equals(n.getProperties().get("build_tool"))); + } + + @Test + void settingsGradleKts_routesToDetectGradleSettings() { + // The Kotlin-DSL syntax `include(":a")` is NOT matched by the detector's current + // regex (which expects a whitespace-separated call — `include ':a'`). This test + // asserts ONLY the dispatch contract: a `settings.gradle.kts` path reaches + // detectGradleSettings and, where the syntax is regex-compatible, produces module + // nodes. + String settingsKts = """ + rootProject.name = "acme" + include ':b' + """; + DetectorContext ctx = DetectorTestUtils.contextFor("settings.gradle.kts", "gradle", settingsKts); + DetectorResult r = detector.detect(ctx); + + assertThat(r.nodes()) + .extracting(CodeNode::getLabel) + .contains("b"); + } + + @Test + void nestedSettingsGradlePath_stillRoutesToDetectGradleSettings() { + // Regression guard: path-like endsWith should still match when the settings file lives in a subdir. + String settings = "include ':core'\n"; + DetectorContext ctx = DetectorTestUtils.contextFor("build/settings.gradle", "gradle", settings); + DetectorResult r = detector.detect(ctx); + + assertThat(r.nodes()) + .extracting(CodeNode::getLabel) + .containsExactly("core"); + } // --------------------------------------------------------------- // Determinism @@ -304,4 +380,11 @@ implementation project(':a') DetectorContext ctx = DetectorTestUtils.contextFor("build.gradle", "gradle", gradle); DetectorTestUtils.assertDeterministic(detector, ctx); } + + @Test + void deterministic_settingsGradle() { + String settings = "include ':a'\ninclude ':b'\n"; + DetectorContext ctx = DetectorTestUtils.contextFor("settings.gradle", "gradle", settings); + DetectorTestUtils.assertDeterministic(detector, ctx); + } } From 02b59403e5a42baece5365165da40c759fe43b7d Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Thu, 23 Apr 2026 14:59:11 +0000 Subject: [PATCH 2/3] refactor(detector): remove dead detectWithAst from ExpressRouteDetector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AbstractTypeScriptDetector#detect() unconditionally dispatches to detectWithRegex, and the class is annotated @DetectorInfo(parser = REGEX), so the protected detectWithAst override (and its three private ANTLR helper methods: extractIdentifierText, extractFirstStringArg, extractStringLiteral) were never invoked. Removed the AST method + helpers and their now-orphaned ANTLR / JavaScriptParser imports. The Python detectors define their own extractFirstStringArg with a distinct signature — unaffected. All 28 ExpressRoute* tests remain green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../typescript/ExpressRouteDetector.java | 96 ------------------- 1 file changed, 96 deletions(-) diff --git a/src/main/java/io/github/randomcodespace/iq/detector/typescript/ExpressRouteDetector.java b/src/main/java/io/github/randomcodespace/iq/detector/typescript/ExpressRouteDetector.java index 893c3cea..28444a8e 100644 --- a/src/main/java/io/github/randomcodespace/iq/detector/typescript/ExpressRouteDetector.java +++ b/src/main/java/io/github/randomcodespace/iq/detector/typescript/ExpressRouteDetector.java @@ -2,17 +2,12 @@ import io.github.randomcodespace.iq.detector.DetectorContext; import io.github.randomcodespace.iq.detector.DetectorResult; -import io.github.randomcodespace.iq.grammar.javascript.JavaScriptParser; -import io.github.randomcodespace.iq.grammar.javascript.JavaScriptParserBaseListener; import io.github.randomcodespace.iq.model.CodeNode; import io.github.randomcodespace.iq.model.NodeKind; -import org.antlr.v4.runtime.tree.ParseTree; -import org.antlr.v4.runtime.tree.ParseTreeWalker; import org.springframework.stereotype.Component; import java.util.ArrayList; import java.util.List; -import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import io.github.randomcodespace.iq.detector.DetectorInfo; @@ -30,10 +25,6 @@ @Component public class ExpressRouteDetector extends AbstractTypeScriptDetector { - private static final Set HTTP_METHODS = Set.of( - "get", "post", "put", "delete", "patch", "options", "head", "all" - ); - private static final Pattern ROUTE_PATTERN = Pattern.compile( "(\\w+)\\.(get|post|put|delete|patch|options|head|all)\\(\\s*['\"`]([^'\"`]+)['\"`]" ); @@ -43,53 +34,6 @@ public String getName() { return "typescript.express_routes"; } - - @Override - protected DetectorResult detectWithAst(ParseTree tree, DetectorContext ctx) { - List nodes = new ArrayList<>(); - String filePath = ctx.filePath(); - String moduleName = ctx.moduleName(); - - ParseTreeWalker.DEFAULT.walk(new JavaScriptParserBaseListener() { - @Override - public void enterArgumentsExpression(JavaScriptParser.ArgumentsExpressionContext argCtx) { - // Look for: expr.method(args) where method is an HTTP method - if (argCtx.singleExpression() instanceof JavaScriptParser.MemberDotExpressionContext memberCtx) { - String methodName = memberCtx.identifierName().getText(); - if (!HTTP_METHODS.contains(methodName)) return; - - String routerName = extractIdentifierText(memberCtx.singleExpression()); - if (routerName == null) return; - - // Get the first string argument (the path) - String path = extractFirstStringArg(argCtx.arguments()); - if (path == null) return; - - String method = methodName.toUpperCase(); - int line = lineOf(argCtx); - - String nodeId = "endpoint:" + (moduleName != null ? moduleName : "") + ":" + method + ":" + path; - CodeNode node = new CodeNode(); - node.setId(nodeId); - node.setKind(NodeKind.ENDPOINT); - node.setLabel(method + " " + path); - node.setFqn(filePath + "::" + method + ":" + path); - node.setModule(moduleName); - node.setFilePath(filePath); - node.setLineStart(line); - node.getProperties().put("protocol", "REST"); - node.getProperties().put("http_method", method); - node.getProperties().put("path_pattern", path); - node.getProperties().put("framework", "express"); - node.getProperties().put("router", routerName); - nodes.add(node); - } - } - }, tree); - - return DetectorResult.of(nodes, List.of()); - } - @Override protected DetectorResult detectWithRegex(DetectorContext ctx) { List nodes = new ArrayList<>(); @@ -124,44 +68,4 @@ protected DetectorResult detectWithRegex(DetectorContext ctx) { return DetectorResult.of(nodes, List.of()); } - - /** Extract a simple identifier name from a single expression, or null. */ - static String extractIdentifierText(JavaScriptParser.SingleExpressionContext expr) { - if (expr instanceof JavaScriptParser.IdentifierExpressionContext idCtx) { - return idCtx.getText(); - } - // For chained access like `this.app`, return the whole text - if (expr instanceof JavaScriptParser.MemberDotExpressionContext memberCtx) { - return memberCtx.getText(); - } - return expr != null ? expr.getText() : null; - } - - /** Extract the first string literal argument from an arguments context. */ - static String extractFirstStringArg(JavaScriptParser.ArgumentsContext args) { - if (args == null || args.argument() == null || args.argument().isEmpty()) return null; - var firstArg = args.argument(0); - if (firstArg == null || firstArg.singleExpression() == null) return null; - var expr = firstArg.singleExpression(); - return extractStringLiteral(expr); - } - - /** Extract a string literal value (strip quotes) from a single expression. */ - static String extractStringLiteral(JavaScriptParser.SingleExpressionContext expr) { - if (expr instanceof JavaScriptParser.LiteralExpressionContext litCtx) { - var literal = litCtx.literal(); - if (literal != null && literal.StringLiteral() != null) { - String raw = literal.StringLiteral().getText(); - return raw.substring(1, raw.length() - 1); - } - } - // Handle template strings (backtick with no expressions) - if (expr instanceof JavaScriptParser.TemplateStringExpressionContext) { - String raw = expr.getText(); - if (raw.startsWith("`") && raw.endsWith("`")) { - return raw.substring(1, raw.length() - 1); - } - } - return null; - } } From 5646a924084578126ad971d68db4553547f55a2a Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Thu, 23 Apr 2026 15:01:20 +0000 Subject: [PATCH 3/3] fix(test): RepositoryIdentityTest no longer depends on local git state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two git-backed tests (resolve_gitRepoWithCommit_commitShaPresent, resolve_detachedHead_branchIsNull) failed when the developer's global gitconfig forced commit signing (commit.gpgsign=true, signingkey set) on a machine without a usable signing key — git commit exited non-zero, the original run() helper ignored the exit code, no commit was made, and rev-parse HEAD returned null. Made the git invocations hermetic: * repo-local config overrides commit.gpgsign / tag.gpgsign to false, unsets core.hooksPath, core.autocrlf, init.templateDir * explicit --no-gpg-sign on the commit (belt-and-braces) * scrub GIT_* env vars on every child process so no ambient CI / worktree state leaks in * run() now asserts the process exited 0 — silent failures become loud test failures * new requireGit() uses Assumptions.assumeTrue to skip cleanly when the git binary is absent (product still covered by non-git tests + RepositoryIdentity's own swallow-on-error path) Verified by running the pre-fix test against a hostile HOME with forced GPG signing — reproduces the 2 failures. Post-fix: 8/8 pass under the same hostile environment. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../intelligence/RepositoryIdentityTest.java | 83 +++++++++++++++++-- 1 file changed, 77 insertions(+), 6 deletions(-) diff --git a/src/test/java/io/github/randomcodespace/iq/intelligence/RepositoryIdentityTest.java b/src/test/java/io/github/randomcodespace/iq/intelligence/RepositoryIdentityTest.java index 1718a4dd..c2f243f4 100644 --- a/src/test/java/io/github/randomcodespace/iq/intelligence/RepositoryIdentityTest.java +++ b/src/test/java/io/github/randomcodespace/iq/intelligence/RepositoryIdentityTest.java @@ -4,13 +4,23 @@ import org.junit.jupiter.api.io.TempDir; import java.nio.file.Path; +import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * Unit tests for {@link RepositoryIdentity}. * Validates graceful degradation when git metadata is unavailable. + * + *

Git invocations in these tests are hermetic: we override every global-config + * setting that can cause {@code git commit} / {@code git init} to fail silently + * on a developer machine but pass on CI (or vice-versa) — most notably + * {@code commit.gpgsign}, {@code tag.gpgsign}, {@code core.hooksPath}, + * {@code init.templateDir}, and {@code core.autocrlf}. We also scrub + * {@code GIT_CONFIG_*} / {@code GIT_DIR} / {@code GIT_WORK_TREE} env vars so the + * invoked processes inherit no ambient git state from the parent. */ class RepositoryIdentityTest { @@ -109,31 +119,92 @@ void record_nullFieldsAllowed() { // Helpers // ------------------------------------------------------------------ + /** + * Pre-flight: skip git-dependent tests when the {@code git} binary is not available. + * Treat absence-of-git as an environment gap, not a product bug — still covered by the + * non-git tests above, and by the {@code RepositoryIdentity.runGit} swallow-all-errors path. + */ + private static void requireGit() { + try { + Process p = new ProcessBuilder("git", "--version") + .redirectErrorStream(true).start(); + boolean ok = p.waitFor() == 0; + assumeTrue(ok, "git binary not available on PATH"); + } catch (Exception e) { + assumeTrue(false, "git binary not available on PATH: " + e.getMessage()); + } + } + private static void initGitRepo(Path dir) throws Exception { - run(dir, "git", "init"); + requireGit(); + // -c overrides are applied to THIS invocation only and cannot be shadowed by a user's + // global gitconfig (unlike `git config` writes into the new .git/config). + run(dir, "git", + "-c", "init.defaultBranch=main", + "-c", "init.templateDir=", + "init"); run(dir, "git", "config", "user.email", "test@test.com"); run(dir, "git", "config", "user.name", "Test"); + // Kill every global knob that can make `git commit` fail on an otherwise-clean repo. + run(dir, "git", "config", "commit.gpgsign", "false"); + run(dir, "git", "config", "tag.gpgsign", "false"); + run(dir, "git", "config", "core.hooksPath", "/dev/null"); + run(dir, "git", "config", "core.autocrlf", "false"); } private static void makeInitialCommit(Path dir) throws Exception { Path readme = dir.resolve("README.md"); java.nio.file.Files.writeString(readme, "# Test"); run(dir, "git", "add", "."); - run(dir, "git", "commit", "-m", "init"); + // --no-gpg-sign is belt-and-braces over the repo-local commit.gpgsign=false set above; + // --allow-empty-message keeps the test robust if a commit.template hook injects content. + run(dir, "git", "-c", "commit.gpgsign=false", "commit", + "--no-gpg-sign", "-m", "init"); } private static String runGit(Path dir, String... args) throws Exception { + requireGit(); var cmd = new java.util.ArrayList(); cmd.add("git"); cmd.addAll(java.util.Arrays.asList(args)); - var proc = new ProcessBuilder(cmd).directory(dir.toFile()).start(); - String out = new String(proc.getInputStream().readAllBytes()).trim(); + var pb = new ProcessBuilder(cmd).directory(dir.toFile()); + scrubGitEnv(pb.environment()); + var proc = pb.start(); + String out; + try (var is = proc.getInputStream()) { + out = new String(is.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8).trim(); + } proc.waitFor(); return out; } + /** + * Execute a git sub-command and assert it exited 0. Fails the test loudly (not silently) + * if setup cannot complete — preferred over ignoring the exit code, which is what let the + * GPG-signing failure slip through before. + */ private static void run(Path dir, String... cmd) throws Exception { - new ProcessBuilder(cmd).directory(dir.toFile()) - .redirectErrorStream(true).start().waitFor(); + var pb = new ProcessBuilder(cmd).directory(dir.toFile()).redirectErrorStream(true); + scrubGitEnv(pb.environment()); + Process proc = pb.start(); + String stderr; + try (var is = proc.getInputStream()) { + stderr = new String(is.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8); + } + int exit = proc.waitFor(); + if (exit != 0) { + throw new IllegalStateException( + "Command failed (exit " + exit + "): " + String.join(" ", cmd) + + "\n" + stderr); + } + } + + /** + * Remove every ambient git env var that could leak the parent shell's context into the + * child process (most commonly {@code GIT_DIR}/{@code GIT_WORK_TREE} in a worktree-based + * setup, or {@code GIT_CONFIG_*} injected by CI runners). + */ + private static void scrubGitEnv(Map env) { + env.keySet().removeIf(k -> k.startsWith("GIT_")); } }