fix: address review findings from PR #1359 (deepagents-cli)

cspell.json

@@ -70,10 +70,10 @@
     "copilotcli",
     "cursorignore",
     "cursorindexingignore",
-    "deepagents",
     "cursorrrules",
     "cursorrules",
     "dbaeumer",
+    "deepagents",
     "deepview",
     "deepwiki",
     "depd",

src/features/hooks/deepagents-hooks.test.ts

@@ -230,5 +230,51 @@ describe("DeepagentsHooks", () => {
       expect(canonical.hooks.sessionStart).toBeDefined();
       expect(canonical.hooks.sessionEnd).toBeDefined();
     });
+
+    it("should skip malformed hook entries", () => {
+      const hooks = new DeepagentsHooks({
+        baseDir: testDir,
+        relativeDirPath: ".deepagents",
+        relativeFilePath: "hooks.json",
+        fileContent: JSON.stringify({
+          hooks: [null, "invalid", { events: ["session.start"] }, { command: [] }],
+        }),
+      });
+
+      const rulesyncHooks = hooks.toRulesyncHooks();
+
+      expect(rulesyncHooks.getJson().hooks).toEqual({});
+    });
+
+    it("should join command parts when bash fallback pattern is not used", () => {
+      const hooks = new DeepagentsHooks({
+        baseDir: testDir,
+        relativeDirPath: ".deepagents",
+        relativeFilePath: "hooks.json",
+        fileContent: JSON.stringify({
+          hooks: [{ command: ["pnpm", "test", "--runInBand"], events: ["task.complete"] }],
+        }),
+      });
+
+      const rulesyncHooks = hooks.toRulesyncHooks();
+
+      expect(rulesyncHooks.getJson().hooks.stop).toEqual([
+        { type: "command", command: "pnpm test --runInBand" },
+      ]);
+    });
+  });
+
+  describe("forDeletion", () => {
+    it("should create a placeholder hooks file for deletion", () => {
+      const hooks = DeepagentsHooks.forDeletion({
+        baseDir: testDir,
+        relativeDirPath: ".deepagents",
+        relativeFilePath: "hooks.json",
+      });
+
+      expect(hooks.getRelativeDirPath()).toBe(".deepagents");
+      expect(hooks.getRelativeFilePath()).toBe("hooks.json");
+      expect(JSON.parse(hooks.getFileContent())).toEqual({ hooks: [] });
+    });
   });
 });

src/features/hooks/deepagents-hooks.ts

@@ -82,6 +82,7 @@ function deepagentsToCanonicalHooks(hooksEntries: DeepagentsHookEntry[]): HooksC
   const canonical: HooksConfig["hooks"] = {};
 
   for (const entry of hooksEntries) {
+    if (typeof entry !== "object" || entry === null) continue;
     if (!Array.isArray(entry.command) || entry.command.length === 0) continue;
 
     // Reconstruct command string: if it's ["bash", "-c", "..."], extract the script

src/features/hooks/hooks-processor.ts

@@ -180,6 +180,9 @@ const toolHooksFactories = new Map<HooksProcessorToolTarget, ToolHooksFactory>([
   ],
 ]);
 
+// Project-mode generation/import should only expose tools that actually write
+// hooks into the workspace. This keeps global-only targets like deepagents out
+// of project target lists while still allowing them in global mode.
 const hooksProcessorToolTargets: ToolTarget[] = [...toolHooksFactories.entries()]
   .filter(([, f]) => f.meta.supportsProject)
   .map(([t]) => t);

src/features/mcp/deepagents-mcp.test.ts

@@ -112,4 +112,40 @@ describe("DeepagentsMcp", () => {
       expect(mcp.getRelativeFilePath()).toBe(".mcp.json");
     });
   });
+
+  describe("toRulesyncMcp", () => {
+    it("should convert deepagents mcp config to rulesync format", () => {
+      const mcp = new DeepagentsMcp({
+        baseDir: testDir,
+        relativeDirPath: ".deepagents",
+        relativeFilePath: ".mcp.json",
+        fileContent: JSON.stringify({
+          mcpServers: {
+            "test-server": { command: "npx", args: ["-y", "test-server"] },
+          },
+          extra: true,
+        }),
+      });
+
+      const rulesyncMcp = mcp.toRulesyncMcp();
+
+      expect(rulesyncMcp.getMcpServers()).toEqual({
+        "test-server": { command: "npx", args: ["-y", "test-server"] },
+      });
+    });
+  });
+
+  describe("forDeletion", () => {
+    it("should create a placeholder file for deletion", () => {
+      const mcp = DeepagentsMcp.forDeletion({
+        baseDir: testDir,
+        relativeDirPath: ".deepagents",
+        relativeFilePath: ".mcp.json",
+      });
+
+      expect(mcp.getRelativeDirPath()).toBe(".deepagents");
+      expect(mcp.getRelativeFilePath()).toBe(".mcp.json");
+      expect(mcp.getFileContent()).toBe("{}");
+    });
+  });
 });

src/features/rules/deepagents-rule.test.ts

@@ -138,6 +138,21 @@ describe("DeepagentsRule", () => {
     });
   });
 
+  describe("forDeletion", () => {
+    it("should create a placeholder root rule for deletion", () => {
+      const rule = DeepagentsRule.forDeletion({
+        baseDir: testDir,
+        relativeDirPath: ".deepagents",
+        relativeFilePath: "AGENTS.md",
+      });
+
+      expect(rule.getRelativeDirPath()).toBe(".deepagents");
+      expect(rule.getRelativeFilePath()).toBe("AGENTS.md");
+      expect(rule.isRoot()).toBe(true);
+      expect(rule.getFileContent()).toBe("");
+    });
+  });
+
   describe("isTargetedByRulesyncRule", () => {
     it("should return true when target is deepagents", () => {
       const rulesyncRule = new RulesyncRule({

src/features/rules/deepagents-rule.ts

@@ -57,6 +57,7 @@ export class DeepagentsRule extends ToolRule {
     relativeFilePath,
     validate = true,
   }: ToolRuleFromFileParams): Promise<DeepagentsRule> {
+    const settablePaths = this.getSettablePaths();
     const isRoot = relativeFilePath === "AGENTS.md";
     const relativePath = isRoot
       ? join(".deepagents", "AGENTS.md")
@@ -66,8 +67,8 @@ export class DeepagentsRule extends ToolRule {
     return new DeepagentsRule({
       baseDir,
       relativeDirPath: isRoot
-        ? this.getSettablePaths().root.relativeDirPath
-        : this.getSettablePaths().nonRoot.relativeDirPath,
+        ? settablePaths.root.relativeDirPath
+        : settablePaths.nonRoot.relativeDirPath,
       relativeFilePath: isRoot ? "AGENTS.md" : relativeFilePath,
       fileContent,
       validate,

src/features/skills/deepagents-skill.test.ts

@@ -28,10 +28,9 @@ describe("DeepagentsSkill", () => {
       expect(paths.relativeDirPath).toBe(join(".deepagents", "skills"));
     });
 
-    it("should throw when global mode requested", () => {
-      expect(() => DeepagentsSkill.getSettablePaths({ global: true })).toThrow(
-        "DeepagentsSkill does not support global mode.",
-      );
+    it("should return project path for global mode", () => {
+      const paths = DeepagentsSkill.getSettablePaths({ global: true });
+      expect(paths.relativeDirPath).toBe(join(".deepagents", "skills"));
     });
   });
 
@@ -119,6 +118,20 @@ Do the thing.`;
       expect(skill.getDirName()).toBe("my-skill");
       expect(skill.getBody()).toBe("Instructions here.");
     });
+
+    it("should omit allowed-tools when deepagents frontmatter is absent", () => {
+      const rulesyncSkill = new RulesyncSkill({
+        baseDir: testDir,
+        relativeDirPath: ".rulesync/skills",
+        dirName: "my-skill",
+        frontmatter: { name: "My Skill", description: "Does something.", targets: ["*"] },
+        body: "Instructions here.",
+      });
+
+      const skill = DeepagentsSkill.fromRulesyncSkill({ baseDir: testDir, rulesyncSkill });
+
+      expect(skill.getFrontmatter()).not.toHaveProperty("allowed-tools");
+    });
   });
 
   describe("isTargetedByRulesyncSkill", () => {
@@ -175,4 +188,18 @@ Do the thing.`;
       expect(rulesyncSkill.getFrontmatter().targets).toEqual(["*"]);
     });
   });
+
+  describe("forDeletion", () => {
+    it("should create a deletable placeholder skill", () => {
+      const skill = DeepagentsSkill.forDeletion({
+        baseDir: testDir,
+        relativeDirPath: join(".deepagents", "skills"),
+        dirName: "my-skill",
+      });
+
+      expect(skill.getRelativeDirPath()).toBe(join(".deepagents", "skills"));
+      expect(skill.getDirName()).toBe("my-skill");
+      expect(skill.getBody()).toBe("");
+    });
+  });
 });

src/features/skills/deepagents-skill.ts

@@ -66,10 +66,7 @@ export class DeepagentsSkill extends ToolSkill {
     }
   }
 
-  static getSettablePaths(options?: { global?: boolean }): ToolSkillSettablePaths {
-    if (options?.global) {
-      throw new Error("DeepagentsSkill does not support global mode.");
-    }
+  static getSettablePaths(_options?: { global?: boolean }): ToolSkillSettablePaths {
     return {
       relativeDirPath: join(".deepagents", "skills"),
     };
@@ -140,7 +137,9 @@ export class DeepagentsSkill extends ToolSkill {
     const deepagentsFrontmatter: DeepagentsSkillFrontmatter = {
       name: rulesyncFrontmatter.name,
       description: rulesyncFrontmatter.description,
-      "allowed-tools": rulesyncFrontmatter.deepagents?.["allowed-tools"],
+      ...(rulesyncFrontmatter.deepagents?.["allowed-tools"] && {
+        "allowed-tools": rulesyncFrontmatter.deepagents["allowed-tools"],
+      }),
     };
 
     return new DeepagentsSkill({