Merge remote-tracking branch 'origin/main' into codex/debug-clawpatch-site

Author: steipete

CHANGELOG.md

@@ -19,7 +19,16 @@
 - Added generic package-less monorepo app-root mapping for Node/Next projects under roots such as `apps/*` and `packages/*` when positive source or framework signals are present.
 - Added Maven project mapping for root, nested, and multi-module Java/Kotlin projects with Spring role slices, Maven validation defaults, and `pom.xml` detection, thanks @julianshess.
 - Added a release-prep checklist for auditing changelog, package metadata, and dry-run package contents without publishing.
+- Improved bounded source grouping so large flat directories split repeated filename families like command, plugin, doctor, and runtime files into more coherent review slices.
+- Fixed acpx provider error reporting by reading the terminal `result.stopReason` envelope and surfacing non-`end_turn` reasons as typed `ClawpatchError` codes (`agent-cancelled`, `agent-refused`, `agent-truncated`) instead of opaque `malformed-output`, thanks @coletebou.
 - Improved OpenCode malformed JSON diagnostics with output length, event kinds, and a bounded preview, thanks @rohitjavvadi.
+- Fixed finding signatures so equivalent evidence remains stable across re-reviews, thanks @rohitjavvadi.
+- Fixed provider exit-code classification for stdout-only authentication and quota failures, thanks @rohitjavvadi.
+- Improved Node route mapping to preserve literal Express and Hono mount prefixes, thanks @rohitjavvadi.
+- Improved Flask route mapping to preserve static blueprint URL prefixes, thanks @rohitjavvadi.
+- Improved Django route mapping to preserve literal `include()` route prefixes, thanks @rohitjavvadi.
+- Added conservative Rails route mapping for literal root and HTTP verb routes, thanks @rohitjavvadi.
+- Fixed heuristic feature mapping to honor configured path include/exclude filters, thanks @schedawg74.
 - Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi.
 - Fixed Laravel route mapping to include array-style `Route::group` prefixes, thanks @rohitjavvadi.
 - Fixed Fastify route-object mapping to emit static method arrays while ignoring dynamic entries, thanks @rohitjavvadi.

README.md

@@ -58,7 +58,8 @@ validation commands and records a patch attempt under `.clawpatch/`.
 - generic extension/plugin packages under workspace roots such as `extensions/*`
   and `plugins/*`, including package metadata, source, docs, and nearby tests
 - semantic Node source groups for large packages, including runtime, commands,
-  auth, storage, monitor, webhook, setup, server, and client slices
+  auth, storage, monitor, webhook, setup, server, client, and repeated filename
+  family slices
 - Nx project metadata from `project.json`, including project-scoped validation
   targets
 - Turborepo task metadata for workspace-aware validation commands and feature

docs/feature-mapping.md

@@ -37,7 +37,8 @@ Supported deterministic mappers today:
 - Nx project metadata from `project.json`, including project names, source roots, project types, and target names
 - Turborepo `turbo.json` metadata for workspace-aware validation commands and feature context
 - bounded Node/TypeScript source groups under `src/`, `lib/`, `app/`,
-  `pages/`, `scripts/`, `server/`, and `api/`
+  `pages/`, `scripts/`, `server/`, and `api/`, with oversized flat
+  directories split by repeated filename families
 - React Router `<Route path element>` declarations and React components in
   root or nested frontend packages such as `frontend/`, `client/`, `web/`,
   workspaces, and packages under `apps/` or `packages/`
@@ -165,12 +166,13 @@ pytest files; Flask `@*.route(...)` handlers; FastAPI `@*.get(...)` /
 `re_path(...)`, and legacy `url(...)` declarations. Flask and FastAPI route
 methods are read from list, tuple, or set literals. FastAPI paths can be
 positional strings or literal `path=` keywords. Django route paths are normalized
-from literal route strings and simple named regex groups; includes are mapped as
-their own URL groups without recursively expanding imported URL configs. Default
+from literal route strings and simple named regex groups, and literal
+`include("module.urls")` routes are expanded under their mount prefixes. Default
 Python command detection covers pytest, ruff, mypy, pyright, and black.
 
 Ruby mapping covers project metadata, executables, source groups, RSpec and
-Minitest suites, and Rails app structure. Rails legacy `config/secrets.yml`,
+Minitest suites, Rails app structure, and literal Rails root and HTTP verb
+routes. Rails legacy `config/secrets.yml`,
 `config/database.yml`, and `config/initializers/secret_token.rb` are not mapped
 as reviewable config because they can contain provider-sensitive secrets.
 

src/agent-mapper.ts

@@ -7,7 +7,14 @@ import { pathExists } from "./fs.js";
 import { runCommandArgs } from "./exec.js";
 import { mapFeatureSeeds, MapResult } from "./mapper.js";
 import { FeatureSeed, SeedFileRef, SeedTestRef } from "./mappers/types.js";
-import { isSafeFile, normalize, shouldSkip, walk } from "./mappers/shared.js";
+import {
+  applyPathFilters,
+  isSafeFile,
+  normalize,
+  PathFilters,
+  shouldSkip,
+  walk,
+} from "./mappers/shared.js";
 
 type AgentMapMode = "heuristic" | "auto" | "agent";
 
@@ -26,15 +33,10 @@ type AgentMapOptions = {
   source: AgentMapMode;
   provider: Provider | null;
   providerOptions: ProviderOptions;
-  inventory?: InventoryFilters;
+  inventory?: PathFilters;
   onProgress?: (event: string, fields: Record<string, string | number | boolean>) => void;
 };
 
-type InventoryFilters = {
-  include: string[];
-  exclude: string[];
-};
-
 type RepoInventorySummary = {
   files: number;
   sourceFiles: number;
@@ -148,6 +150,7 @@ export async function mapWithSource(
     options.provider,
     options.providerOptions,
     inventory,
+    options.inventory,
   );
   options.onProgress?.("agent-done", {
     features: agent.features.length,
@@ -199,6 +202,7 @@ async function agentMap(
   provider: Provider,
   providerOptions: ProviderOptions,
   inventory: RepoInventory,
+  filters: PathFilters | undefined,
 ): Promise<MapResult> {
   const prompt = buildAgentMapPrompt(project, {
     manifests: inventory.manifests,
@@ -212,12 +216,10 @@ async function agentMap(
   const seeds = await Promise.all(
     output.features.map((feature) => toSeed(root, feature, inventory.allFiles)),
   );
-  return mapFeatureSeeds(
-    root,
-    project,
-    existing,
-    uniqueSeeds(seeds.filter((seed): seed is FeatureSeed => seed !== null)),
-  );
+  const mappedSeeds = uniqueSeeds(seeds.filter((seed): seed is FeatureSeed => seed !== null));
+  return filters === undefined
+    ? mapFeatureSeeds(root, project, existing, mappedSeeds)
+    : mapFeatureSeeds(root, project, existing, mappedSeeds, { filters });
 }
 
 async function toSeed(
@@ -385,10 +387,10 @@ async function repoInventory(
   root: string,
   project: ProjectRecord,
   features: FeatureRecord[],
-  filters: InventoryFilters | undefined,
+  filters: PathFilters | undefined,
 ): Promise<RepoInventory> {
   const skipPath = await inventorySkipPath(root, project, features);
-  const files = applyInventoryFilters(
+  const files = applyPathFilters(
     ((await gitInventoryFiles(root)) ?? (await walk(root, [""], skipPath))).filter(
       (path) => !skipPath(path),
     ),
@@ -443,73 +445,12 @@ async function gitInventoryFiles(root: string): Promise<string[] | null> {
   return existing.filter((path): path is string => path !== null);
 }
 
-function applyInventoryFilters(files: string[], filters: InventoryFilters | undefined): string[] {
-  if (filters === undefined) {
-    return files;
-  }
-  return files.filter(
-    (file) =>
-      filters.include.some((pattern) => inventoryPatternMatches(pattern, file)) &&
-      !filters.exclude.some((pattern) => inventoryPatternMatches(pattern, file)),
-  );
-}
-
 function isInventoryPath(path: string): boolean {
   return (
     path.length > 0 && !isAbsolute(path) && !path.includes("\0") && !path.split("/").includes("..")
   );
 }
 
-function inventoryPatternMatches(pattern: string, path: string): boolean {
-  const normalized = pattern.replace(/\\/gu, "/").replace(/^\.\//u, "");
-  if (normalized === "**" || normalized === "**/*") {
-    return true;
-  }
-  if (normalized.length === 0) {
-    return false;
-  }
-  if (!/[?*]/u.test(normalized)) {
-    return path === normalized || path.startsWith(`${normalized}/`);
-  }
-  if (normalized.endsWith("/**")) {
-    const prefix = normalized.slice(0, -3);
-    if (/[?*]/u.test(prefix)) {
-      return new RegExp(`^${globPatternRegExp(prefix)}(?:/.*)?$`, "u").test(path);
-    }
-    return prefix.length === 0 || path === prefix || path.startsWith(`${prefix}/`);
-  }
-  return new RegExp(`^${globPatternRegExp(normalized)}$`, "u").test(path);
-}
-
-function globPatternRegExp(pattern: string): string {
-  let source = "";
-  for (let index = 0; index < pattern.length; index += 1) {
-    const char = pattern[index];
-    if (char === "*") {
-      if (pattern[index + 1] === "*") {
-        if (pattern[index + 2] === "/") {
-          source += "(?:.*/)?";
-          index += 2;
-        } else {
-          source += ".*";
-          index += 1;
-        }
-      } else {
-        source += "[^/]*";
-      }
-    } else if (char === "?") {
-      source += "[^/]";
-    } else {
-      source += regexpEscape(char ?? "");
-    }
-  }
-  return source;
-}
-
-function regexpEscape(value: string): string {
-  return value.replace(/[\\^$.*+?()[\]{}|]/gu, "\\$&");
-}
-
 async function inventorySkipPath(
   root: string,
   project: ProjectRecord,

src/app.ts

@@ -123,12 +123,14 @@ export async function mapCommand(
   const config = applyProviderFlags(loaded.config, flags);
   const provider = source === "heuristic" ? null : providerByName(config.provider.name);
   const existing = await readFeatures(loaded.paths);
+  const filters = { include: config.include, exclude: config.exclude };
   emitProgress(context, "map", "start", {
     source,
     existing: existing.length,
     dryRun: flags["dryRun"] === true,
   });
   const heuristic = await mapFeatures(loaded.root, loaded.project, existing, {
+    filters,
     onProgress: (event) => {
       emitProgress(context, "map", event.event, {
         mapper: event.mapper,
@@ -149,7 +151,7 @@ export async function mapCommand(
     source,
     provider,
     providerOptions: providerOptions(config),
-    inventory: { include: config.include, exclude: config.exclude },
+    inventory: filters,
     onProgress: (event, fields) => {
       emitProgress(context, "map", event, fields);
     },

src/findings.test.ts

@@ -0,0 +1,127 @@
+import { describe, expect, it } from "vitest";
+import { findingFromOutput } from "./findings.js";
+import type { ReviewOutput } from "./types.js";
+
+describe("findingFromOutput", () => {
+  it("keeps signatures stable for equivalent evidence with different key insertion order", () => {
+    const orderedEvidence = {
+      path: "src/app.ts",
+      startLine: 10,
+      endLine: 12,
+      symbol: "runWorkflow",
+      quote: "await runWorkflow();",
+    };
+    const reorderedEvidence = {} as Record<string, unknown>;
+    reorderedEvidence["quote"] = "await runWorkflow();";
+    reorderedEvidence["symbol"] = "runWorkflow";
+    reorderedEvidence["endLine"] = 12;
+    reorderedEvidence["startLine"] = 10;
+    reorderedEvidence["path"] = "src/app.ts";
+
+    const first = findingFromOutput(finding([orderedEvidence]), "feature_1", "run_1");
+    const second = findingFromOutput(
+      finding([reorderedEvidence as ReviewOutput["findings"][number]["evidence"][number]]),
+      "feature_1",
+      "run_1",
+    );
+
+    expect(second.signature).toBe(first.signature);
+    expect(second.findingId).toBe(first.findingId);
+  });
+
+  it("keeps signatures stable for equivalent evidence with different reference order", () => {
+    const first = findingFromOutput(
+      finding([
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflow();",
+        },
+        {
+          path: "src/provider.ts",
+          startLine: 20,
+          endLine: 22,
+          symbol: null,
+          quote: null,
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+    const second = findingFromOutput(
+      finding([
+        {
+          path: "src/provider.ts",
+          startLine: 20,
+          endLine: 22,
+          symbol: null,
+          quote: null,
+        },
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflow();",
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+
+    expect(second.signature).toBe(first.signature);
+    expect(second.findingId).toBe(first.findingId);
+  });
+
+  it("keeps signatures distinct when evidence content changes", () => {
+    const first = findingFromOutput(
+      finding([
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflow();",
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+    const second = findingFromOutput(
+      finding([
+        {
+          path: "src/app.ts",
+          startLine: 10,
+          endLine: 12,
+          symbol: "runWorkflow",
+          quote: "await runWorkflowSafely();",
+        },
+      ]),
+      "feature_1",
+      "run_1",
+    );
+
+    expect(second.signature).not.toBe(first.signature);
+    expect(second.findingId).not.toBe(first.findingId);
+  });
+});
+
+function finding(
+  evidence: ReviewOutput["findings"][number]["evidence"],
+): ReviewOutput["findings"][number] {
+  return {
+    title: "Finding title",
+    category: "bug",
+    severity: "medium",
+    confidence: "high",
+    evidence,
+    reasoning: "Reasoning.",
+    reproduction: null,
+    recommendation: "Recommendation.",
+    whyTestsDoNotAlreadyCoverThis: "No regression coverage exists.",
+    suggestedRegressionTest: null,
+    minimumFixScope: "Keep the fix narrow.",
+  };
+}