fix: address review findings — sort mutation, injection, directory baseline, error handling

- C1: use [...array].sort() to avoid mutating fingerprint inputs
- C2: replace echo piping with here-string to prevent shell injection in hooks
- M1/M2: add allKnownFiles param to classifyUpdate for correct directory baseline
- M3: add try-catch to loadFingerprints for corrupted JSON resilience
- M5: add hasStructuralAnalysis flag for conservative no-tree-sitter classification
- m3: replace fragile sed with node JSON.parse in SessionStart hook
- Add 7 new tests covering all fixes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

understand-anything-plugin/hooks/hooks.json

@@ -6,7 +6,7 @@
         "hooks": [
           {
             "type": "command",
-            "command": "echo \"$TOOL_INPUT\" | grep -qE 'git\\s+(commit|merge|cherry-pick|rebase)' && [ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/knowledge-graph.json ] && echo '[understand-anything] Commit detected with auto-update enabled. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to incrementally update the knowledge graph. Do not ask the user for confirmation — just do it.' || true"
+            "command": "grep -qE 'git\\s+(commit|merge|cherry-pick|rebase)' <<< \"$TOOL_INPUT\" && [ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/knowledge-graph.json ] && echo '[understand-anything] Commit detected with auto-update enabled. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to incrementally update the knowledge graph. Do not ask the user for confirmation — just do it.' || true"
           }
         ]
       }
@@ -16,7 +16,7 @@
         "hooks": [
           {
             "type": "command",
-            "command": "[ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/meta.json ] && [ -f .understand-anything/knowledge-graph.json ] && [ \"$(sed -n 's/.*\"gitCommitHash\".*\"\\([^\"]*\\)\".*/\\1/p' .understand-anything/meta.json)\" != \"$(git rev-parse HEAD 2>/dev/null)\" ] && echo '[understand-anything] Knowledge graph is stale. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to check for structural changes and update the graph. Do not ask the user for confirmation — just do it.' || true"
+            "command": "[ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/meta.json ] && [ -f .understand-anything/knowledge-graph.json ] && [ \"$(node -p \"JSON.parse(require('fs').readFileSync('.understand-anything/meta.json','utf8')).gitCommitHash\")\" != \"$(git rev-parse HEAD 2>/dev/null)\" ] && echo '[understand-anything] Knowledge graph is stale. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to check for structural changes and update the graph. Do not ask the user for confirmation — just do it.' || true"
           }
         ]
       }

understand-anything-plugin/packages/core/src/__tests__/change-classifier.test.ts

@@ -46,7 +46,9 @@ describe("classifyUpdate", () => {
       cosmeticOnlyFiles: ["src/d.ts"],
     });
 
-    const decision = classifyUpdate(analysis, 50);
+    // src/ already exists in the project, so adding src/c.ts is not a directory change
+    const allKnownFiles = ["src/a.ts", "src/b.ts", "src/d.ts", "lib/util.ts"];
+    const decision = classifyUpdate(analysis, 50, allKnownFiles);
 
     expect(decision.action).toBe("PARTIAL_UPDATE");
     expect(decision.filesToReanalyze).toEqual(["src/a.ts", "src/b.ts", "src/c.ts"]);
@@ -73,7 +75,8 @@ describe("classifyUpdate", () => {
       newFiles: ["newdir/file.ts"],
     });
 
-    const decision = classifyUpdate(analysis, 50);
+    const allKnownFiles = ["src/existing.ts", "src/other.ts", "lib/util.ts"];
+    const decision = classifyUpdate(analysis, 50, allKnownFiles);
 
     expect(decision.action).toBe("ARCHITECTURE_UPDATE");
     expect(decision.rerunArchitecture).toBe(true);
@@ -85,7 +88,34 @@ describe("classifyUpdate", () => {
       deletedFiles: ["olddir/removed.ts"],
     });
 
-    const decision = classifyUpdate(analysis, 50);
+    const allKnownFiles = ["src/existing.ts", "src/other.ts"];
+    const decision = classifyUpdate(analysis, 50, allKnownFiles);
+
+    expect(decision.action).toBe("ARCHITECTURE_UPDATE");
+    expect(decision.rerunArchitecture).toBe(true);
+  });
+
+  it("does NOT trigger ARCHITECTURE_UPDATE for new file in existing directory", () => {
+    const analysis = makeAnalysis({
+      newFiles: ["src/newfile.ts"],
+    });
+
+    // src/ is already known via other files in the project
+    const allKnownFiles = ["src/a.ts", "src/b.ts", "lib/util.ts"];
+    const decision = classifyUpdate(analysis, 50, allKnownFiles);
+
+    expect(decision.action).toBe("PARTIAL_UPDATE");
+    expect(decision.rerunArchitecture).toBe(false);
+  });
+
+  it("triggers ARCHITECTURE_UPDATE for new file in genuinely new directory", () => {
+    const analysis = makeAnalysis({
+      newFiles: ["brand-new-pkg/index.ts"],
+    });
+
+    // allKnownFiles only contains src/ and lib/ — no brand-new-pkg/
+    const allKnownFiles = ["src/a.ts", "src/b.ts", "lib/util.ts"];
+    const decision = classifyUpdate(analysis, 50, allKnownFiles);
 
     expect(decision.action).toBe("ARCHITECTURE_UPDATE");
     expect(decision.rerunArchitecture).toBe(true);

understand-anything-plugin/packages/core/src/__tests__/fingerprint.test.ts

@@ -136,6 +136,7 @@ describe("compareFingerprints", () => {
     imports: [{ source: "./utils", specifiers: ["format"] }],
     exports: ["main"],
     totalLines: 30,
+    hasStructuralAnalysis: true,
   };
 
   it("returns NONE when content hash is identical", () => {
@@ -242,6 +243,7 @@ describe("compareFingerprints", () => {
       ...baseFp,
       contentHash: "different",
       classes: [{ name: "MyClass", methods: ["init"], properties: [], exported: true, lineCount: 30 }],
+      hasStructuralAnalysis: true,
     };
     const result = compareFingerprints(baseFp, withClass);
     expect(result.changeLevel).toBe("STRUCTURAL");
@@ -252,16 +254,79 @@ describe("compareFingerprints", () => {
     const oldFp: FileFingerprint = {
       ...baseFp,
       classes: [{ name: "Foo", methods: ["a", "b"], properties: [], exported: true, lineCount: 30 }],
+      hasStructuralAnalysis: true,
     };
     const newFp: FileFingerprint = {
       ...baseFp,
       contentHash: "different",
       classes: [{ name: "Foo", methods: ["a", "c"], properties: [], exported: true, lineCount: 30 }],
+      hasStructuralAnalysis: true,
     };
     const result = compareFingerprints(oldFp, newFp);
     expect(result.changeLevel).toBe("STRUCTURAL");
     expect(result.details).toContain("methods changed: Foo");
   });
+
+  it("does NOT mutate input arrays (sort must use spread-copy)", () => {
+    const oldFp: FileFingerprint = {
+      ...baseFp,
+      classes: [{ name: "Foo", methods: ["b", "a"], properties: ["y", "x"], exported: true, lineCount: 30 }],
+      imports: [{ source: "./utils", specifiers: ["z", "a"] }],
+      hasStructuralAnalysis: true,
+    };
+    const newFp: FileFingerprint = {
+      ...baseFp,
+      contentHash: "different",
+      classes: [{ name: "Foo", methods: ["b", "a"], properties: ["y", "x"], exported: true, lineCount: 30 }],
+      imports: [{ source: "./utils", specifiers: ["z", "a"] }],
+      hasStructuralAnalysis: true,
+    };
+
+    // Snapshot original order before comparison
+    const oldMethodsBefore = [...oldFp.classes[0].methods];
+    const oldPropertiesBefore = [...oldFp.classes[0].properties];
+    const oldSpecifiersBefore = [...oldFp.imports[0].specifiers];
+    const newMethodsBefore = [...newFp.classes[0].methods];
+    const newPropertiesBefore = [...newFp.classes[0].properties];
+    const newSpecifiersBefore = [...newFp.imports[0].specifiers];
+
+    compareFingerprints(oldFp, newFp);
+
+    // Arrays must remain in their original order (not sorted in-place)
+    expect(oldFp.classes[0].methods).toEqual(oldMethodsBefore);
+    expect(oldFp.classes[0].properties).toEqual(oldPropertiesBefore);
+    expect(oldFp.imports[0].specifiers).toEqual(oldSpecifiersBefore);
+    expect(newFp.classes[0].methods).toEqual(newMethodsBefore);
+    expect(newFp.classes[0].properties).toEqual(newPropertiesBefore);
+    expect(newFp.imports[0].specifiers).toEqual(newSpecifiersBefore);
+  });
+
+  it("classifies as STRUCTURAL when hasStructuralAnalysis is false (no tree-sitter)", () => {
+    const oldFp: FileFingerprint = {
+      filePath: "config.yaml",
+      contentHash: "hash_old",
+      functions: [],
+      classes: [],
+      imports: [],
+      exports: [],
+      totalLines: 10,
+      hasStructuralAnalysis: false,
+    };
+    const newFp: FileFingerprint = {
+      filePath: "config.yaml",
+      contentHash: "hash_new",
+      functions: [],
+      classes: [],
+      imports: [],
+      exports: [],
+      totalLines: 12,
+      hasStructuralAnalysis: false,
+    };
+
+    const result = compareFingerprints(oldFp, newFp);
+    expect(result.changeLevel).toBe("STRUCTURAL");
+    expect(result.details).toContain("no structural analysis available — conservative classification");
+  });
 });
 
 describe("analyzeChanges", () => {
@@ -282,6 +347,7 @@ describe("analyzeChanges", () => {
         imports: [],
         exports: ["main"],
         totalLines: 30,
+        hasStructuralAnalysis: true,
       },
       "src/utils.ts": {
         filePath: "src/utils.ts",
@@ -291,6 +357,7 @@ describe("analyzeChanges", () => {
         imports: [],
         exports: [],
         totalLines: 10,
+        hasStructuralAnalysis: true,
       },
     },
   };

understand-anything-plugin/packages/core/src/change-classifier.ts

@@ -21,6 +21,7 @@ export interface UpdateDecision {
 export function classifyUpdate(
   analysis: ChangeAnalysis,
   totalFilesInGraph: number,
+  allKnownFiles: string[] = [],
 ): UpdateDecision {
   const { newFiles, deletedFiles, structurallyChangedFiles, cosmeticOnlyFiles, unchangedFiles } = analysis;
   const structuralCount = structurallyChangedFiles.length + newFiles.length + deletedFiles.length;
@@ -53,7 +54,7 @@ export function classifyUpdate(
   }
 
   // Check if directory structure changed (new/deleted top-level directories)
-  const hasDirectoryChanges = detectDirectoryChanges(newFiles, deletedFiles, structurallyChangedFiles);
+  const hasDirectoryChanges = detectDirectoryChanges(newFiles, deletedFiles, allKnownFiles);
 
   if (hasDirectoryChanges || structuralCount > 10) {
     return {
@@ -79,15 +80,16 @@ export function classifyUpdate(
 
 /**
  * Detect if the changes affect the directory structure (new or removed directories).
- * Checks if any new/deleted files introduce or remove a top-level source directory.
+ * Uses all known files in the project as the baseline for existing directories,
+ * then checks if any new/deleted files introduce or remove a top-level source directory.
  */
 function detectDirectoryChanges(
   newFiles: string[],
   deletedFiles: string[],
-  structurallyChangedFiles: string[],
+  allKnownFiles: string[],
 ): boolean {
   const existingDirs = new Set(
-    structurallyChangedFiles.map((f) => topDirectory(f)).filter(Boolean),
+    allKnownFiles.map((f) => topDirectory(f)).filter(Boolean),
   );
 
   for (const f of newFiles) {

understand-anything-plugin/packages/core/src/fingerprint.ts

@@ -35,6 +35,7 @@ export interface FileFingerprint {
   imports: ImportFingerprint[];
   exports: string[];
   totalLines: number;
+  hasStructuralAnalysis: boolean;
 }
 
 export interface FingerprintStore {
@@ -116,6 +117,7 @@ export function extractFileFingerprint(
     imports,
     exports,
     totalLines,
+    hasStructuralAnalysis: true,
   };
 }
 
@@ -137,6 +139,16 @@ export function compareFingerprints(
     return { filePath: newFp.filePath, changeLevel: "NONE", details: [] };
   }
 
+  // Conservative path: if either fingerprint lacks structural analysis,
+  // we cannot verify structure didn't change — classify as STRUCTURAL.
+  if (!oldFp.hasStructuralAnalysis || !newFp.hasStructuralAnalysis) {
+    return {
+      filePath: newFp.filePath,
+      changeLevel: "STRUCTURAL",
+      details: ["no structural analysis available — conservative classification"],
+    };
+  }
+
   // Compare function signatures
   const oldFuncNames = new Set(oldFp.functions.map((f) => f.name));
   const newFuncNames = new Set(newFp.functions.map((f) => f.name));
@@ -194,10 +206,10 @@ export function compareFingerprints(
     const oldCls = oldFp.classes.find((c) => c.name === newCls.name);
     if (!oldCls) continue;
 
-    if (JSON.stringify(oldCls.methods.sort()) !== JSON.stringify(newCls.methods.sort())) {
+    if (JSON.stringify([...oldCls.methods].sort()) !== JSON.stringify([...newCls.methods].sort())) {
       details.push(`methods changed: ${newCls.name}`);
     }
-    if (JSON.stringify(oldCls.properties.sort()) !== JSON.stringify(newCls.properties.sort())) {
+    if (JSON.stringify([...oldCls.properties].sort()) !== JSON.stringify([...newCls.properties].sort())) {
       details.push(`properties changed: ${newCls.name}`);
     }
     if (oldCls.exported !== newCls.exported) {
@@ -206,8 +218,8 @@ export function compareFingerprints(
   }
 
   // Compare imports
-  const oldImports = oldFp.imports.map((i) => `${i.source}:${i.specifiers.sort().join(",")}`).sort();
-  const newImports = newFp.imports.map((i) => `${i.source}:${i.specifiers.sort().join(",")}`).sort();
+  const oldImports = oldFp.imports.map((i) => `${i.source}:${[...i.specifiers].sort().join(",")}`).sort();
+  const newImports = newFp.imports.map((i) => `${i.source}:${[...i.specifiers].sort().join(",")}`).sort();
 
   if (JSON.stringify(oldImports) !== JSON.stringify(newImports)) {
     details.push("imports changed");
@@ -265,6 +277,7 @@ export function buildFingerprintStore(
         imports: [],
         exports: [],
         totalLines: content.split("\n").length,
+        hasStructuralAnalysis: false,
       };
     }
   }
@@ -341,6 +354,7 @@ export function analyzeChanges(
         imports: [],
         exports: [],
         totalLines: content.split("\n").length,
+        hasStructuralAnalysis: false,
       };
     }
 

understand-anything-plugin/packages/core/src/persistence/index.ts

@@ -64,7 +64,11 @@ export function saveFingerprints(projectRoot: string, store: FingerprintStore):
 export function loadFingerprints(projectRoot: string): FingerprintStore | null {
   const filePath = join(projectRoot, UA_DIR, FINGERPRINT_FILE);
   if (!existsSync(filePath)) return null;
-  return JSON.parse(readFileSync(filePath, "utf-8")) as FingerprintStore;
+  try {
+    return JSON.parse(readFileSync(filePath, "utf-8")) as FingerprintStore;
+  } catch {
+    return null;
+  }
 }
 
 const DEFAULT_CONFIG: ProjectConfig = { autoUpdate: false };

understand-anything-plugin/packages/core/src/persistence/persistence.test.ts

@@ -2,8 +2,10 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest";
 import { mkdtempSync, rmSync, existsSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
-import { saveGraph, loadGraph, saveMeta, loadMeta } from "./index.js";
+import { writeFileSync } from "node:fs";
+import { saveGraph, loadGraph, saveMeta, loadMeta, saveFingerprints, loadFingerprints } from "./index.js";
 import type { KnowledgeGraph, AnalysisMeta } from "../types.js";
+import type { FingerprintStore } from "../fingerprint.js";
 
 describe("persistence", () => {
   let tempDir: string;
@@ -115,4 +117,46 @@ describe("persistence", () => {
       expect(loaded).toBeNull();
     });
   });
+
+  describe("saveFingerprints / loadFingerprints", () => {
+    const sampleFingerprints: FingerprintStore = {
+      version: "1.0.0",
+      gitCommitHash: "abc123",
+      generatedAt: "2026-03-14T00:00:00.000Z",
+      files: {
+        "src/index.ts": {
+          filePath: "src/index.ts",
+          contentHash: "deadbeef",
+          functions: [],
+          classes: [],
+          imports: [],
+          exports: [],
+          totalLines: 10,
+          hasStructuralAnalysis: false,
+        },
+      },
+    };
+
+    it("should round-trip fingerprints correctly", () => {
+      saveFingerprints(tempDir, sampleFingerprints);
+      const loaded = loadFingerprints(tempDir);
+
+      expect(loaded).toEqual(sampleFingerprints);
+    });
+
+    it("should return null when no fingerprints file exists", () => {
+      const loaded = loadFingerprints(tempDir);
+      expect(loaded).toBeNull();
+    });
+
+    it("should return null when fingerprints.json is corrupted", () => {
+      const dir = join(tempDir, ".understand-anything");
+      // Ensure the directory exists by saving first, then overwrite with garbage
+      saveFingerprints(tempDir, sampleFingerprints);
+      writeFileSync(join(dir, "fingerprints.json"), "{{not valid json!!", "utf-8");
+
+      const loaded = loadFingerprints(tempDir);
+      expect(loaded).toBeNull();
+    });
+  });
 });