feat: warn when ADR file scope is too broad

.archgate/adrs/ARCH-005-testing-standards.md

@@ -45,6 +45,7 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes
 - Test public module interfaces, not private implementation details
 - Use descriptive test names that explain the expected behavior
 - **When mocking `fetch` in tests, assign directly to `globalThis.fetch`** — use `globalThis.fetch = mockFn as unknown as typeof fetch`. Restore in `afterEach` via `mock.restore()` (from `bun:test`) or by reassigning the original reference before the test.
+- **Wrap `spyOn` / `mockImplementation` calls in `try/finally` to guarantee `mockRestore()` runs** — when `expect()` assertions fail, they throw immediately, skipping any `mockRestore()` that follows. The un-restored spy leaks into subsequent tests, causing false positives or false negatives. Pattern: `const spy = spyOn(...).mockImplementation(() => {}); try { /* assertions */ } finally { spy.mockRestore(); }`. Alternatively, create and restore spies in `beforeEach`/`afterEach` hooks instead of inline.
 
 ### Don't
 
@@ -57,6 +58,7 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes
 - Don't skip tests without a tracking issue
 - Don't import test utilities from `node:test` — use Bun's built-in `bun:test` module
 - **Don't use `mock.module("node:fetch", ...)` to intercept HTTP fetch calls** — in Bun, the runtime fetch is `globalThis.fetch` and `mock.module` targeting `node:fetch` does not intercept it. The mock silently has no effect: the real network is hit, making tests non-deterministic and dependent on external services. Assign `globalThis.fetch` directly instead (see Do's above).
+- **Don't place `mockRestore()` after assertions without `try/finally` protection** — if the assertion throws, the spy is never restored and contaminates subsequent tests. This is especially dangerous when spying on globals like `console.warn` or `globalThis.fetch`, where a leaked spy silently suppresses or redirects output for every test that follows.
 
 ## Implementation Pattern
 
@@ -209,6 +211,7 @@ Code reviewers MUST verify:
 4. Tests that instantiate SDK objects (servers, clients, connections) manage their lifecycle in `beforeEach`/`afterEach`, not inside individual test bodies
 5. Tests that call `git commit` on a temp repo configure `user.email` and `user.name` locally before committing
 6. Tests that mock HTTP fetch assign `globalThis.fetch` directly — no `mock.module("node:fetch", ...)` usage
+7. Tests that use `spyOn` or `mockImplementation` inline (not in `beforeEach`/`afterEach`) wrap the spy lifecycle in `try/finally` to guarantee `mockRestore()` runs even when assertions fail
 
 ## References
 

src/engine/git-files.ts

@@ -4,6 +4,11 @@
 
 import { logDebug, logWarn } from "../helpers/log";
 
+/** Warn when an ADR's resolved file scope exceeds this many files. */
+export const SCOPE_FILE_WARN_THRESHOLD = 1000;
+/** Warn when the glob scan phase takes longer than this (milliseconds). */
+export const SCOPE_SCAN_WARN_MS = 2000;
+
 /**
  * Run a git command using Bun.spawn (cross-platform, no shell).
  * Bun.$ hangs on Windows due to pipe handling issues — this is the safe alternative.
@@ -79,6 +84,7 @@ export async function resolveScopedFiles(
     ? await getGitTrackedFiles(projectRoot)
     : null;
 
+  const scanStart = performance.now();
   const results = await Promise.all(
     patterns.map(async (pattern) => {
       const glob = new Bun.Glob(pattern);
@@ -94,8 +100,16 @@ export async function resolveScopedFiles(
       return files;
     })
   );
+  const scanMs = performance.now() - scanStart;
 
   const all = [...new Set(results.flat())].sort();
+
+  if (all.length > SCOPE_FILE_WARN_THRESHOLD || scanMs > SCOPE_SCAN_WARN_MS) {
+    logWarn(
+      `${label}: Resolved ${all.length} files from patterns: ${patterns.join(", ")} (scan took ${Math.round(scanMs)}ms). Consider narrowing the \`files\` patterns in the ADR frontmatter to improve performance.`
+    );
+  }
+
   logDebug(
     "Scoped files resolved:",
     all.length,

tests/engine/git-files.test.ts

@@ -10,6 +10,7 @@ import {
   getStagedFiles,
   getChangedFiles,
   resolveScopedFiles,
+  SCOPE_FILE_WARN_THRESHOLD,
 } from "../../src/engine/git-files";
 import { git, safeRmSync } from "../test-utils";
 
@@ -211,5 +212,60 @@ describe("git-files", () => {
       expect(warnCalls.some((msg) => msg.includes("ADR BUILD-001"))).toBe(true);
       warnSpy.mockRestore();
     });
+
+    test("warns when file scope exceeds threshold", async () => {
+      await git(["init"], tempDir);
+      mkdirSync(join(tempDir, "src"), { recursive: true });
+      const fileCount = SCOPE_FILE_WARN_THRESHOLD + 1;
+      for (let i = 0; i < fileCount; i++) {
+        writeFileSync(
+          join(tempDir, "src", `file-${String(i).padStart(4, "0")}.ts`),
+          `export const x${i} = ${i};`
+        );
+      }
+      await git(["add", "."], tempDir);
+      const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
+      try {
+        const files = await resolveScopedFiles(tempDir, ["src/**/*.ts"], {
+          adrId: "SCOPE-001",
+        });
+        expect(files.length).toBeGreaterThan(SCOPE_FILE_WARN_THRESHOLD);
+        const warnCalls = warnSpy.mock.calls.map((args) => args.join(" "));
+        expect(
+          warnCalls.some(
+            (msg) =>
+              msg.includes("ADR SCOPE-001") &&
+              msg.includes(`${fileCount} files`) &&
+              msg.includes("scan took") &&
+              msg.includes("Consider narrowing")
+          )
+        ).toBe(true);
+      } finally {
+        warnSpy.mockRestore();
+      }
+    }, 30_000);
+
+    test("does not warn when file scope is within threshold", async () => {
+      await git(["init"], tempDir);
+      mkdirSync(join(tempDir, "src"), { recursive: true });
+      for (let i = 0; i < 10; i++) {
+        writeFileSync(
+          join(tempDir, "src", `file-${i}.ts`),
+          `export const x${i} = ${i};`
+        );
+      }
+      await git(["add", "."], tempDir);
+      const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
+      try {
+        const files = await resolveScopedFiles(tempDir, ["src/**/*.ts"]);
+        expect(files.length).toBeLessThanOrEqual(SCOPE_FILE_WARN_THRESHOLD);
+        const warnCalls = warnSpy.mock.calls.map((args) => args.join(" "));
+        expect(
+          warnCalls.some((msg) => msg.includes("Consider narrowing"))
+        ).toBe(false);
+      } finally {
+        warnSpy.mockRestore();
+      }
+    });
   });
 });