feat(permission): integrate auto-review and hard guards

pi-permission-system/test/auto-review-ledger.test.ts

@@ -0,0 +1,141 @@
+import { mkdtempSync, readFileSync, rmSync, statSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+
+import {
+  DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG,
+  DEFAULT_AUTO_REVIEW_CONFIG,
+} from "#src/auto-review/config";
+import { AutoReviewLedger } from "#src/auto-review/ledger";
+import type { PromptPermissionDetails } from "#src/service";
+
+const roots: string[] = [];
+
+afterEach(() => {
+  for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
+});
+
+function details(requestId: string): PromptPermissionDetails {
+  return {
+    requestId,
+    source: "tool",
+    surface: "bash",
+    toolName: "bash",
+    command: "npm install example",
+    agentName: "main",
+  } as unknown as PromptPermissionDetails;
+}
+
+describe("AutoReviewLedger", () => {
+  it("marks analysis pending only after the configured number of completed verdicts", () => {
+    const root = mkdtempSync(join(tmpdir(), "auto-review-ledger-"));
+    roots.push(root);
+    const ledger = new AutoReviewLedger(root);
+    const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 2 };
+    const assessment = {
+      outcome: "allow" as const,
+      riskLevel: "low" as const,
+      userAuthorization: "high" as const,
+      rationale: "directly authorized",
+    };
+
+    ledger.recordDecision({
+      details: details("request-1"),
+      assessment,
+      policyRevision: "policy-a",
+      config: DEFAULT_AUTO_REVIEW_CONFIG,
+      analysis,
+    });
+    expect(ledger.hasPendingAnalysis()).toBe(false);
+    expect(ledger.prepareBatch(analysis)).toBeUndefined();
+
+    ledger.recordDecision({
+      details: details("request-2"),
+      assessment,
+      policyRevision: "policy-a",
+      config: DEFAULT_AUTO_REVIEW_CONFIG,
+      analysis,
+    });
+    expect(ledger.hasPendingAnalysis()).toBe(true);
+    const batch = ledger.prepareBatch(analysis);
+    expect(batch?.fromSequence).toBe(1);
+    expect(batch?.toSequence).toBe(2);
+    expect(batch?.records).toHaveLength(2);
+    expect(batch?.records[0]).not.toHaveProperty("rationale");
+    expect(batch?.records[0].normalizedPattern).toBe("npm install *");
+  });
+
+  it("persists owner-only state and resumes a pending batch", () => {
+    const root = mkdtempSync(join(tmpdir(), "auto-review-ledger-"));
+    roots.push(root);
+    const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 1 };
+    const ledger = new AutoReviewLedger(root);
+    ledger.recordDecision({
+      details: details("request-1"),
+      assessment: {
+        outcome: "deny",
+        riskLevel: "high",
+        userAuthorization: "unknown",
+        rationale: "unsafe",
+      },
+      policyRevision: "policy-b",
+      config: DEFAULT_AUTO_REVIEW_CONFIG,
+      analysis,
+    });
+
+    const resumed = new AutoReviewLedger(root);
+    expect(resumed.hasPendingAnalysis()).toBe(true);
+    expect(resumed.prepareBatch(analysis)?.records[0].outcome).toBe("deny");
+    expect(statSync(join(root, "state.json")).mode & 0o777).toBe(0o600);
+    expect(readFileSync(join(root, "ledger", "decisions.jsonl"), "utf8")).not.toContain("unsafe");
+  });
+
+  it("does not retain command arguments, secret path segments, or rationale", () => {
+    const root = mkdtempSync(join(tmpdir(), "auto-review-ledger-"));
+    roots.push(root);
+    const ledger = new AutoReviewLedger(root);
+    const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 10 };
+    const assessment = {
+      outcome: "allow" as const,
+      riskLevel: "medium" as const,
+      userAuthorization: "high" as const,
+      rationale: "secret-rationale",
+    };
+    const commandRecord = ledger.recordDecision({
+      details: {
+        requestId: "request-command",
+        source: "tool",
+        agentName: "main",
+        surface: "bash",
+        toolName: "bash",
+        command: "curl https://token.example/?api_key=secret-value",
+      } as unknown as PromptPermissionDetails,
+      assessment,
+      policyRevision: "policy-c",
+      config: DEFAULT_AUTO_REVIEW_CONFIG,
+      analysis,
+    });
+    const pathRecord = ledger.recordDecision({
+      details: {
+        requestId: "request-path",
+        source: "tool",
+        agentName: "main",
+        surface: "path",
+        toolName: "read",
+        path: "clients/acme-secret/token.txt",
+      } as unknown as PromptPermissionDetails,
+      assessment,
+      policyRevision: "policy-c",
+      config: DEFAULT_AUTO_REVIEW_CONFIG,
+      analysis,
+    });
+
+    expect(commandRecord.normalizedPattern).toBe("curl *");
+    expect(pathRecord.normalizedPattern).toBe("<relative>/*");
+    const persisted = readFileSync(join(root, "ledger", "decisions.jsonl"), "utf8");
+    expect(persisted).not.toContain("secret-value");
+    expect(persisted).not.toContain("acme-secret");
+    expect(persisted).not.toContain("secret-rationale");
+  });
+});

pi-permission-system/test/auto-review-runtime.test.ts

@@ -0,0 +1,180 @@
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import type { ExtensionAPI, ModelRegistry, SessionManager } from "@earendil-works/pi-coding-agent";
+import {
+  DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG,
+  DEFAULT_AUTO_REVIEW_CONFIG,
+} from "#src/auto-review/config";
+import { AutoReviewLedger } from "#src/auto-review/ledger";
+import { installBuiltinAutoReview } from "#src/auto-review/runtime";
+import { getAnalysisStateDir } from "#src/config-paths";
+import type { ConfigReader } from "#src/config-store";
+import type { PermissionsService, PromptPermissionDetails } from "#src/service";
+import type { ReviewLogger } from "#src/session-logger";
+
+type AnalysisRunner = NonNullable<
+  Parameters<typeof installBuiltinAutoReview>[0]["runAnalysis"]
+>;
+
+const roots: string[] = [];
+
+afterEach(() => {
+  for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
+});
+
+function seedPending(agentDir: string) {
+  const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 1 };
+  const ledger = new AutoReviewLedger(getAnalysisStateDir(agentDir));
+  ledger.recordDecision({
+    details: {
+      requestId: "request-1",
+      source: "tool",
+      agentName: "main",
+      surface: "bash",
+      toolName: "bash",
+      command: "npm install example",
+    } as unknown as PromptPermissionDetails,
+    assessment: {
+      outcome: "allow",
+      riskLevel: "low",
+      userAuthorization: "high",
+      rationale: "directly authorized",
+    },
+    policyRevision: "policy-a",
+    config: DEFAULT_AUTO_REVIEW_CONFIG,
+    analysis,
+  });
+  return { analysis, ledger };
+}
+
+function harness(agentDir: string, analysis: typeof DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, runAnalysis: Parameters<typeof installBuiltinAutoReview>[0]["runAnalysis"]) {
+  const sendMessage = vi.fn();
+  const registerAuthorizer = vi.fn(() => () => {});
+  const review = vi.fn();
+  const controller = installBuiltinAutoReview({
+    pi: { sendMessage } as unknown as ExtensionAPI,
+    service: { registerAuthorizer } as unknown as PermissionsService,
+    config: {
+      current: () => ({
+        debugLog: false,
+        permissionReviewLog: true,
+        yoloMode: false,
+        doublePressToConfirm: true,
+        autoReview: DEFAULT_AUTO_REVIEW_CONFIG,
+        analysis,
+      }),
+    } as ConfigReader,
+    logger: { review } as unknown as ReviewLogger,
+    agentDir,
+    runAnalysis,
+  });
+  controller.handleSessionStart({
+    modelRegistry: {} as ModelRegistry,
+    sessionManager: { getBranch: () => [] } as unknown as Pick<SessionManager, "getBranch">,
+  });
+  return { controller, sendMessage, registerAuthorizer, review };
+}
+
+describe("built-in AutoReview lifecycle", () => {
+  it("analyzes a persisted pending checkpoint at agent_settled and follows up in the same session", async () => {
+    const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
+    roots.push(agentDir);
+    const { analysis } = seedPending(agentDir);
+    const runAnalysis = vi.fn<AnalysisRunner>(async () => ({
+      summary: "No unconditional policy changes recommended.",
+      allowCandidates: [],
+      denyCandidates: [],
+      keepReview: [],
+      hardGuardCandidates: [],
+    }));
+    const { controller, sendMessage, registerAuthorizer } = harness(agentDir, analysis, runAnalysis);
+
+    await controller.handleAgentSettled();
+
+    expect(registerAuthorizer).toHaveBeenCalledWith("auto-review", expect.any(Function));
+    expect(runAnalysis).toHaveBeenCalledTimes(1);
+    expect(runAnalysis.mock.calls[0]?.[0].batch).toMatchObject({ fromSequence: 1, toSequence: 1 });
+    expect(sendMessage).toHaveBeenCalledWith(
+      expect.objectContaining({ customType: "permission-policy-analysis-ready", display: true }),
+      { deliverAs: "followUp", triggerTurn: true },
+    );
+    expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(false);
+
+    await controller.handleAgentSettled();
+    expect(runAnalysis).toHaveBeenCalledTimes(1);
+  });
+
+  it("keeps the checkpoint pending when background analysis fails", async () => {
+    const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
+    roots.push(agentDir);
+    const { analysis } = seedPending(agentDir);
+    const runAnalysis = vi.fn<AnalysisRunner>(async () => {
+      throw new Error("analysis unavailable");
+    });
+    const { controller, sendMessage, review } = harness(agentDir, analysis, runAnalysis);
+
+    await controller.handleAgentSettled();
+
+    expect(sendMessage).not.toHaveBeenCalled();
+    expect(review).toHaveBeenCalledWith("auto_review.analysis_failed", { error: "analysis unavailable" });
+    expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(true);
+  });
+
+  it("discards a stale result after session replacement without advancing the checkpoint", async () => {
+    const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
+    roots.push(agentDir);
+    const { analysis } = seedPending(agentDir);
+    let resolveAnalysis!: (proposal: Awaited<ReturnType<AnalysisRunner>>) => void;
+    const runAnalysis = vi.fn<AnalysisRunner>(() => new Promise((resolve) => {
+      resolveAnalysis = resolve;
+    }));
+    const { controller, sendMessage, review } = harness(agentDir, analysis, runAnalysis);
+
+    const pending = controller.handleAgentSettled();
+    expect(runAnalysis).toHaveBeenCalledTimes(1);
+    controller.handleSessionShutdown();
+    controller.handleSessionStart({
+      modelRegistry: {} as ModelRegistry,
+      sessionManager: { getBranch: () => [] } as unknown as Pick<SessionManager, "getBranch">,
+    });
+    resolveAnalysis({
+      summary: "stale proposal",
+      allowCandidates: [],
+      denyCandidates: [],
+      keepReview: [],
+      hardGuardCandidates: [],
+    });
+    await pending;
+
+    expect(sendMessage).not.toHaveBeenCalled();
+    expect(review).not.toHaveBeenCalledWith("auto_review.analysis_completed", expect.anything());
+    expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(true);
+  });
+
+  it("does not advance the checkpoint when same-session delivery fails", async () => {
+    const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
+    roots.push(agentDir);
+    const { analysis } = seedPending(agentDir);
+    const runAnalysis = vi.fn<AnalysisRunner>(async () => ({
+      summary: "proposal",
+      allowCandidates: [],
+      denyCandidates: [],
+      keepReview: [],
+      hardGuardCandidates: [],
+    }));
+    const { controller, sendMessage, review } = harness(agentDir, analysis, runAnalysis);
+    sendMessage.mockImplementation(() => {
+      throw new Error("session delivery unavailable");
+    });
+
+    await controller.handleAgentSettled();
+
+    expect(review).toHaveBeenCalledWith("auto_review.analysis_failed", {
+      error: "session delivery unavailable",
+    });
+    expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(true);
+  });
+});

pi-permission-system/test/composition-root.test.ts

@@ -51,14 +51,16 @@ const SERVING_REGISTRY_KEY = Symbol.for(
   "@gotgenes/pi-permission-system:serving-registry",
 );
 
-/** The six events the factory must register a handler for. */
+/** Events the factory must register a handler for. */
 const EXPECTED_HANDLERS = [
+  "agent_settled",
   "before_agent_start",
   "input",
   "resources_discover",
   "session_shutdown",
   "session_start",
   "tool_call",
+  "turn_start",
 ];
 
 let agentDir: string;
@@ -102,7 +104,7 @@ function writeProjectConfig(
   const dir = join(cwd, ".pi", "extensions", "pi-permission-system");
   mkdirSync(dir, { recursive: true });
   writeFileSync(
-    join(dir, "config.json"),
+    join(dir, "policy.jsonc"),
     `${JSON.stringify(config, null, 2)}\n`,
     "utf8",
   );

pi-permission-system/test/config-loader.test.ts

@@ -736,13 +736,13 @@ describe("loadAndMergeConfigs", () => {
   function writeGlobal(content: Record<string, unknown>): void {
     const dir = join(agentDir, "extensions", "pi-permission-system");
     mkdirSync(dir, { recursive: true });
-    writeFileSync(join(dir, "config.json"), JSON.stringify(content));
+    writeFileSync(join(dir, "policy.jsonc"), JSON.stringify(content));
   }
 
   function writeProject(content: Record<string, unknown>): void {
     const dir = join(cwd, ".pi", "extensions", "pi-permission-system");
     mkdirSync(dir, { recursive: true });
-    writeFileSync(join(dir, "config.json"), JSON.stringify(content));
+    writeFileSync(join(dir, "policy.jsonc"), JSON.stringify(content));
   }
 
   function writeLegacyGlobalPolicy(content: Record<string, unknown>): void {
@@ -826,7 +826,7 @@ describe("loadAndMergeConfigs", () => {
     expect(result.merged.yoloMode).toBe(true);
   });
 
-  it("does not emit legacy extension config issue when path equals new global path", () => {
+  it("does not emit legacy extension config issue when its path equals the runtime legacy path", () => {
     const newGlobalDir = join(agentDir, "extensions", "pi-permission-system");
     mkdirSync(newGlobalDir, { recursive: true });
     writeFileSync(

pi-permission-system/test/config-paths.test.ts

@@ -26,9 +26,9 @@ describe("config-paths", () => {
       );
     });
 
-    it("getGlobalConfigPath returns config.json under the global config dir", () => {
+    it("getGlobalConfigPath returns policy.jsonc under the global config dir", () => {
       expect(getGlobalConfigPath(agentDir)).toBe(
-        join(agentDir, "extensions", "pi-permission-system", "config.json"),
+        join(agentDir, "extensions", "pi-permission-system", "policy.jsonc"),
       );
     });
 
@@ -38,9 +38,9 @@ describe("config-paths", () => {
       );
     });
 
-    it("getProjectConfigPath returns .pi/extensions/pi-permission-system/config.json under cwd", () => {
+    it("getProjectConfigPath returns .pi/extensions/pi-permission-system/policy.jsonc under cwd", () => {
       expect(getProjectConfigPath(cwd)).toBe(
-        join(cwd, ".pi", "extensions", "pi-permission-system", "config.json"),
+        join(cwd, ".pi", "extensions", "pi-permission-system", "policy.jsonc"),
       );
     });
 

pi-permission-system/test/config-pipeline.test.ts

@@ -38,7 +38,7 @@ describe("config pipeline seam", () => {
   function writeGlobal(content: Record<string, unknown>): void {
     const dir = join(agentDir, "extensions", "pi-permission-system");
     mkdirSync(dir, { recursive: true });
-    writeFileSync(join(dir, "config.json"), JSON.stringify(content));
+    writeFileSync(join(dir, "policy.jsonc"), JSON.stringify(content));
   }
 
   it("a runtime knob survives the full pipeline", () => {
@@ -99,7 +99,7 @@ describe("config pipeline seam", () => {
     const projectDir = join(cwd, ".pi", "extensions", "pi-permission-system");
     mkdirSync(projectDir, { recursive: true });
     writeFileSync(
-      join(projectDir, "config.json"),
+      join(projectDir, "policy.jsonc"),
       JSON.stringify({ toolInputPreviewMaxLength: 500 }),
     );
 

pi-permission-system/test/hard-guard-loader.test.ts

@@ -0,0 +1,53 @@
+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+
+import { HardGuardPolicyLoader } from "#src/hard-guard-loader";
+
+const roots: string[] = [];
+
+afterEach(() => {
+  for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
+});
+
+describe("HardGuardPolicyLoader", () => {
+  it("loads only the global JSONC hard-guard file and preserves allow exceptions", () => {
+    const root = mkdtempSync(join(tmpdir(), "hard-guard-loader-"));
+    roots.push(root);
+    const userPath = join(root, "hard-guards.jsonc");
+    mkdirSync(root, { recursive: true });
+    writeFileSync(userPath, `{
+      // Allow public examples, deny the rest of the credentials tree.
+      "version": 1,
+      "permission": {
+        "*": "allow",
+        "~/.ssh/**": "deny",
+        "~/.ssh/*.example": "allow"
+      }
+    }`);
+
+    const loader = new HardGuardPolicyLoader(userPath, join(root, "missing-fallback.jsonc"));
+    const config = loader.loadGlobalConfig();
+
+    expect(config.permission).toEqual({
+      "*": "allow",
+      "~/.ssh/**": "deny",
+      "~/.ssh/*.example": "allow",
+    });
+    expect(loader.loadProjectConfig()).toEqual({});
+    expect(loader.getConfigIssues()).toEqual([]);
+  });
+
+  it("fails closed when neither the user file nor fallback is valid", () => {
+    const root = mkdtempSync(join(tmpdir(), "hard-guard-loader-"));
+    roots.push(root);
+    const userPath = join(root, "hard-guards.jsonc");
+    writeFileSync(userPath, `{ "version": 1, "permission": { "*": "ask" } }`);
+
+    const loader = new HardGuardPolicyLoader(userPath, join(root, "missing-fallback.jsonc"));
+
+    expect(loader.loadGlobalConfig().permission).toEqual({ "*": "deny" });
+    expect(loader.getConfigIssues().join("\n")).toContain("allow or deny");
+  });
+});

pi-permission-system/test/permission-resolver.test.ts

@@ -327,4 +327,32 @@ describe("PermissionResolver", () => {
       expect(result).toEqual(["issue-1"]);
     });
   });
+
+  describe("hard-guard precedence", () => {
+    it("returns a hard deny before ordinary policy or session rules can allow it", () => {
+      const ordinary = makePermissionManager();
+      const hardGuard = makePermissionManager();
+      vi.mocked(hardGuard.check).mockReturnValue({
+        state: "deny",
+        toolName: "read",
+        source: "special",
+        origin: "global",
+        matchedPattern: "~/.ssh/**",
+      });
+      vi.mocked(hardGuard.getToolPermission).mockReturnValue("deny");
+      vi.mocked(hardGuard.getConfigIssues).mockReturnValue(["hard-guard issue"]);
+      const resolver = new PermissionResolver(
+        ordinary,
+        { getRuleset: () => [] },
+        hardGuard,
+      );
+
+      const result = resolver.checkPermission("read", { path: "~/.ssh/id_ed25519" });
+
+      expect(result.state).toBe("deny");
+      expect(ordinary.check).not.toHaveBeenCalled();
+      expect(resolver.getToolPermission("read")).toBe("deny");
+      expect(resolver.getConfigIssues()).toContain("hard-guard issue");
+    });
+  });
 });