fix(review): bound manifest path instruction globs

src/signals/focus-manifest.ts

@@ -144,6 +144,8 @@ export type PreMergeCheck = {
 
 // A hard cap so a hostile/huge manifest can't bloat the reviewer prompt (mirrors REVIEW_FIELD_KEYS discipline).
 const MAX_PATH_INSTRUCTIONS = 50;
+const MAX_REVIEW_PATH_GUIDANCE_LENGTH = 4_000;
+const CONTROL_CHARACTER_PATTERN = /[\u0000-\u001f\u007f]/;
 
 /**
  * Normalized maintainer focus manifest. Repo owners declare which work areas are wanted,
@@ -662,8 +664,8 @@ function parseReviewExcludePaths(value: JsonValue | undefined, warnings: string[
 }
 
 /** Parse `review.path_instructions` — an array of `{ path, instructions }` entries. Each must have a non-empty
- *  string `path` (a manifest glob) and PUBLIC-SAFE string `instructions`; invalid/unsafe entries are dropped with
- *  a warning. Capped at MAX_PATH_INSTRUCTIONS so a huge manifest can't bloat the reviewer prompt. */
+ *  bounded string `path` (a manifest glob) and PUBLIC-SAFE string `instructions`; invalid/unsafe entries are
+ *  dropped with a warning. Capped at MAX_PATH_INSTRUCTIONS so a huge manifest can't bloat the reviewer prompt. */
 function parseReviewPathInstructions(value: JsonValue | undefined, warnings: string[]): ReviewPathInstruction[] {
   if (value === undefined || value === null) return [];
   if (!Array.isArray(value)) {
@@ -681,15 +683,19 @@ function parseReviewPathInstructions(value: JsonValue | undefined, warnings: str
       continue;
     }
     const e = entry as Record<string, JsonValue>;
-    const path = typeof e.path === "string" ? e.path.trim() : "";
+    let path = typeof e.path === "string" ? e.path.trim() : "";
     if (!path) {
       warnings.push(`Manifest "review.path_instructions[${index}].path" must be a non-empty string; ignoring the entry.`);
       continue;
     }
-    if (path.length > MAX_ITEM_LENGTH) {
-      warnings.push(`Manifest "review.path_instructions[${index}].path" exceeds ${MAX_ITEM_LENGTH} chars; ignoring the entry.`);
+    if (CONTROL_CHARACTER_PATTERN.test(path)) {
+      warnings.push(`Manifest "review.path_instructions[${index}].path" must not contain control characters; ignoring the entry.`);
       continue;
     }
+    if (path.length > MAX_ITEM_LENGTH) {
+      warnings.push(`Manifest "review.path_instructions[${index}].path" truncated an over-long entry.`);
+      path = path.slice(0, MAX_ITEM_LENGTH);
+    }
     if (e.instructions === undefined || e.instructions === null) {
       warnings.push(`Manifest "review.path_instructions[${index}].instructions" is required; ignoring the entry.`);
       continue;
@@ -749,10 +755,32 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
  */
 export function resolveReviewPathInstructions(pathInstructions: ReviewPathInstruction[], changedPaths: string[]): string {
   if (pathInstructions.length === 0 || changedPaths.length === 0) return "";
-  const applicable = pathInstructions.filter((entry) => changedPaths.some((path) => matchesManifestPath(path, entry.path)));
+  const normalizedChangedPaths = changedPaths.map(normalizePathForMatch).filter(Boolean);
+  if (normalizedChangedPaths.length === 0) return "";
+  const applicable = pathInstructions.filter((entry) => {
+    const matches = compileManifestPathMatcher(entry.path);
+    return normalizedChangedPaths.some((path) => matches(path));
+  });
   if (applicable.length === 0) return "";
-  const lines = applicable.map((entry) => `- \`${entry.path}\`: ${entry.instructions}`);
-  return `\n\nPath-specific review instructions from the maintainer — apply these to the changed files that match each glob:\n${lines.join("\n")}`;
+
+  const header = "\n\nPath-specific review instructions from the maintainer — apply these to the changed files that match each glob:";
+  const lines: string[] = [];
+  let length = header.length;
+  let omitted = 0;
+  for (const entry of applicable) {
+    const line = `- \`${entry.path}\`: ${entry.instructions}`;
+    const nextLength = length + 1 + line.length;
+    if (nextLength > MAX_REVIEW_PATH_GUIDANCE_LENGTH) {
+      omitted += 1;
+      continue;
+    }
+    lines.push(line);
+    length = nextLength;
+  }
+  if (lines.length === 0) return "";
+  const omittedLine = omitted > 0 ? `\n- (${omitted} additional matching path instruction(s) omitted to keep the reviewer prompt bounded.)` : "";
+  const guidance = `${header}\n${lines.join("\n")}${omittedLine}`;
+  return guidance.length > MAX_REVIEW_PATH_GUIDANCE_LENGTH ? guidance.slice(0, MAX_REVIEW_PATH_GUIDANCE_LENGTH) : guidance;
 }
 
 /** Resolve the AI-reviewer overrides (`review.profile` + `review.path_instructions` + `review.exclude_paths`) from

test/unit/focus-manifest.test.ts

@@ -1129,7 +1129,7 @@ describe("parseFocusManifest review config", () => {
           "nope", // non-mapping → dropped
           { path: "y/**" }, // missing instructions → dropped
           { path: 42, instructions: "non-string path" }, // path not a string → dropped
-          { path: `${"a".repeat(400)}/x`, instructions: "over-long path" }, // > MAX_ITEM_LENGTH → dropped (#review-audit)
+          { path: "bad\npath", instructions: "control path" }, // control character path → dropped
         ],
       },
     });
@@ -1142,7 +1142,7 @@ describe("parseFocusManifest review config", () => {
     expect(m.warnings.some((w) => /path_instructions\[4\]/.test(w))).toBe(true);
     expect(m.warnings.some((w) => /path_instructions\[5\]\.instructions/.test(w))).toBe(true);
     expect(m.warnings.some((w) => /path_instructions\[6\]\.path/.test(w))).toBe(true); // non-string path
-    expect(m.warnings.some((w) => /path_instructions\[7\]\.path.*exceeds/.test(w))).toBe(true); // over-long path
+    expect(m.warnings.some((w) => /path_instructions\[7\]\.path.*control characters/.test(w))).toBe(true);
     // Round-trips through the cache serializer.
     expect(parseFocusManifest({ review: reviewConfigToJson(m.review) }).review.pathInstructions).toEqual(m.review.pathInstructions);
   });
@@ -1159,6 +1159,15 @@ describe("parseFocusManifest review config", () => {
     expect(m.review.pathInstructions).toHaveLength(50);
     expect(m.warnings.some((w) => /path_instructions.*capped/.test(w))).toBe(true);
   });
+
+  it("caps review.path_instructions paths at the manifest item length with a warning", () => {
+    const longGlob = `src/${"a".repeat(400)}/**`;
+    const m = parseFocusManifest({ review: { path_instructions: [{ path: longGlob, instructions: "keep it bounded" }] } });
+    expect(m.review.pathInstructions).toHaveLength(1);
+    expect(m.review.pathInstructions[0]?.path).toHaveLength(300);
+    expect(m.review.pathInstructions[0]?.path).toBe(longGlob.slice(0, 300));
+    expect(m.warnings.some((w) => /path_instructions\[0\]\.path.*over-long/.test(w))).toBe(true);
+  });
 });
 
 describe("resolveReviewPathInstructions (#review-path-instructions)", () => {
@@ -1186,6 +1195,19 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => {
     expect(out).toContain("Cover both branches.");
   });
 
+  it("bounds the resolved path guidance prompt section", () => {
+    const many = Array.from({ length: 50 }, (_, i) => ({ path: `src/${i}/**`, instructions: "x".repeat(300) }));
+    const changedPaths = many.map((entry) => `${entry.path.slice(0, -3)}/file.ts`);
+    const out = resolveReviewPathInstructions(many, changedPaths);
+    expect(out.length).toBeLessThanOrEqual(4_000);
+    expect(out).toContain("Path-specific review instructions");
+    expect(out).toContain("omitted to keep the reviewer prompt bounded");
+  });
+
+  it("returns an empty string when changed paths normalize to blanks", () => {
+    expect(resolveReviewPathInstructions(rules, ["", "///"])).toBe("");
+  });
+
   it("resolveReviewPromptOverrides: non-null manifest passes the config through; null manifest → defaults", () => {
     const manifest = parseFocusManifest({ review: { profile: "chill", inline_comments: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], exclude_paths: ["**/*.lock"] } });
     expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", inlineComments: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], excludePaths: ["**/*.lock"] });