fix(codexcli): address review findings from #1602

src/features/permissions/codexcli-permissions.test.ts

@@ -119,6 +119,37 @@ default_permissions = "rulesync"
     expect(json.permission.webfetch?.["example.com"]).toBe("deny");
   });
 
+  it("should not set glob_scan_max_depth when project-root globs contain only single-level wildcards", async () => {
+    const logger = createMockLogger();
+    const rulesyncPermissions = new RulesyncPermissions({
+      outputRoot: testDir,
+      relativeDirPath: ".rulesync",
+      relativeFilePath: "permissions.json",
+      fileContent: JSON.stringify({
+        permission: {
+          read: {
+            "src/*": "allow",
+          },
+          write: {
+            "docs/*": "allow",
+          },
+        },
+      }),
+    });
+
+    const codexPermissions = await CodexcliPermissions.fromRulesyncPermissions({
+      outputRoot: testDir,
+      rulesyncPermissions,
+      logger,
+    });
+
+    const fileContent = codexPermissions.getFileContent();
+    expect(fileContent).toContain('[permissions.rulesync.filesystem.":project_roots"]');
+    expect(fileContent).toContain('"src/*" = "read"');
+    expect(fileContent).toContain('"docs/*" = "write"');
+    expect(fileContent).not.toContain("glob_scan_max_depth");
+  });
+
   it("should import nested Codex project root filesystem rules", () => {
     const codexPermissions = new CodexcliPermissions({
       outputRoot: testDir,
@@ -148,6 +179,61 @@ glob_scan_max_depth = 8
     expect(json.permission.edit?.["docs/**"]).toBe("allow");
   });
 
+  it("should warn when :project_roots is set as a direct string access rule", async () => {
+    const logger = createMockLogger();
+    const rulesyncPermissions = new RulesyncPermissions({
+      outputRoot: testDir,
+      relativeDirPath: ".rulesync",
+      relativeFilePath: "permissions.json",
+      fileContent: JSON.stringify({
+        permission: {
+          read: {
+            ":project_roots": "deny",
+            "src/**": "allow",
+          },
+        },
+      }),
+    });
+
+    await CodexcliPermissions.fromRulesyncPermissions({
+      outputRoot: testDir,
+      rulesyncPermissions,
+      logger,
+    });
+
+    expect(logger.warn).toHaveBeenCalledWith(
+      expect.stringContaining('":project_roots" is set as a direct filesystem access rule'),
+    );
+  });
+
+  it("should skip empty string patterns with a warning", async () => {
+    const logger = createMockLogger();
+    const rulesyncPermissions = new RulesyncPermissions({
+      outputRoot: testDir,
+      relativeDirPath: ".rulesync",
+      relativeFilePath: "permissions.json",
+      fileContent: JSON.stringify({
+        permission: {
+          read: {
+            "": "allow",
+            "src/**": "allow",
+          },
+        },
+      }),
+    });
+
+    const codexPermissions = await CodexcliPermissions.fromRulesyncPermissions({
+      outputRoot: testDir,
+      rulesyncPermissions,
+      logger,
+    });
+
+    expect(logger.warn).toHaveBeenCalledWith("Skipping empty pattern in filesystem permissions.");
+
+    const fileContent = codexPermissions.getFileContent();
+    expect(fileContent).not.toContain('""');
+  });
+
   it("should load existing .codex/config.toml", async () => {
     const codexDir = join(testDir, ".codex");
     await ensureDir(codexDir);

src/features/permissions/codexcli-permissions.ts

@@ -19,7 +19,7 @@ import {
 const RULESYNC_PROFILE_NAME = "rulesync";
 const RULESYNC_BASH_RULES_FILE_NAME = "rulesync.rules";
 const CODEX_PROJECT_ROOTS_KEY = ":project_roots";
-const CODEX_GLOB_SCAN_MAX_DEPTH = 8;
+const CODEX_GLOB_SCAN_MAX_DEPTH = 8; // Matches Codex CLI default glob_scan_max_depth
 
 type CodexFilesystemAccess = "read" | "write" | "none";
 type CodexFilesystemRuleTable = Record<string, CodexFilesystemAccess>;
@@ -180,6 +180,7 @@ function convertRulesyncToCodexProfile({
           projectRootFilesystem,
           pattern,
           access: mapReadAction(action),
+          logger,
         });
       }
       continue;
@@ -192,6 +193,7 @@ function convertRulesyncToCodexProfile({
           projectRootFilesystem,
           pattern,
           access: mapWriteAction(action),
+          logger,
         });
       }
       continue;
@@ -216,6 +218,11 @@ function convertRulesyncToCodexProfile({
   }
 
   if (Object.keys(projectRootFilesystem).length > 0) {
+    if (typeof filesystem[CODEX_PROJECT_ROOTS_KEY] === "string") {
+      logger?.warn(
+        `"${CODEX_PROJECT_ROOTS_KEY}" is set as a direct filesystem access rule in the permissions, but it will be overwritten by project-root rules. Consider removing the direct "${CODEX_PROJECT_ROOTS_KEY}" entry.`,
+      );
+    }
     if (Object.keys(projectRootFilesystem).some((pattern) => pattern.includes("**"))) {
       filesystem.glob_scan_max_depth = CODEX_GLOB_SCAN_MAX_DEPTH;
     }
@@ -275,12 +282,19 @@ function addFilesystemRule({
   projectRootFilesystem,
   pattern,
   access,
+  logger,
 }: {
   filesystem: CodexFilesystem;
   projectRootFilesystem: CodexFilesystemRuleTable;
   pattern: string;
   access: CodexFilesystemAccess;
+  logger?: ToolPermissionsFromRulesyncPermissionsParams["logger"];
 }): void {
+  if (pattern.trim() === "") {
+    logger?.warn("Skipping empty pattern in filesystem permissions.");
+    return;
+  }
+
   if (canBeCodexFilesystemRoot(pattern)) {
     filesystem[pattern] = access;
     return;