feat: allow read-only shell commands in plan mode (#413)

Co-authored-by: Letta <noreply@letta.com>

src/permissions/mode.ts

@@ -1,6 +1,8 @@
 // src/permissions/mode.ts
 // Permission mode management (default, acceptEdits, plan, bypassPermissions)
 
+import { isReadOnlyShellCommand } from "./readOnlyShell";
+
 export type PermissionMode =
   | "default"
   | "acceptEdits"
@@ -195,6 +197,23 @@ class PermissionModeManager {
           }
         }
 
+        // Allow read-only shell commands (ls, git status, git log, etc.)
+        const shellTools = [
+          "Bash",
+          "shell",
+          "Shell",
+          "shell_command",
+          "ShellCommand",
+          "run_shell_command",
+          "RunShellCommand",
+        ];
+        if (shellTools.includes(toolName)) {
+          const command = toolArgs?.command as string | string[] | undefined;
+          if (command && isReadOnlyShellCommand(command)) {
+            return "allow";
+          }
+        }
+
         // Everything else denied in plan mode
         return "deny";
       }

src/tests/permissions-mode.test.ts

@@ -277,7 +277,7 @@ test("plan mode - denies Write", () => {
   expect(result.reason).toContain("Plan mode is active");
 });
 
-test("plan mode - denies Bash", () => {
+test("plan mode - denies non-read-only Bash", () => {
   permissionMode.setMode("plan");
 
   const permissions: PermissionRules = {
@@ -288,7 +288,7 @@ test("plan mode - denies Bash", () => {
 
   const result = checkPermission(
     "Bash",
-    { command: "ls" },
+    { command: "npm install" },
     permissions,
     "/Users/test/project",
   );
@@ -297,6 +297,53 @@ test("plan mode - denies Bash", () => {
   expect(result.matchedRule).toBe("plan mode");
 });
 
+test("plan mode - allows read-only Bash commands", () => {
+  permissionMode.setMode("plan");
+
+  const permissions: PermissionRules = {
+    allow: [],
+    deny: [],
+    ask: [],
+  };
+
+  // ls should be allowed
+  const lsResult = checkPermission(
+    "Bash",
+    { command: "ls -la" },
+    permissions,
+    "/Users/test/project",
+  );
+  expect(lsResult.decision).toBe("allow");
+  expect(lsResult.matchedRule).toBe("plan mode");
+
+  // git status should be allowed
+  const gitStatusResult = checkPermission(
+    "Bash",
+    { command: "git status" },
+    permissions,
+    "/Users/test/project",
+  );
+  expect(gitStatusResult.decision).toBe("allow");
+
+  // git log should be allowed
+  const gitLogResult = checkPermission(
+    "Bash",
+    { command: "git log --oneline -10" },
+    permissions,
+    "/Users/test/project",
+  );
+  expect(gitLogResult.decision).toBe("allow");
+
+  // git diff should be allowed
+  const gitDiffResult = checkPermission(
+    "Bash",
+    { command: "git diff HEAD~1" },
+    permissions,
+    "/Users/test/project",
+  );
+  expect(gitDiffResult.decision).toBe("allow");
+});
+
 test("plan mode - denies WebFetch", () => {
   permissionMode.setMode("plan");
 
@@ -354,9 +401,10 @@ test("Permission mode takes precedence over CLI allowedTools", () => {
     ask: [],
   };
 
+  // Use a non-read-only command to test precedence
   const result = checkPermission(
     "Bash",
-    { command: "ls" },
+    { command: "npm install" },
     permissions,
     "/Users/test/project",
   );