fix: address review findings for the checks feature

- Merge the tool-scoped 'amp:' frontmatter section with precedence over
  canonical values (per feature-change-guidelines), exclude all
  tool-target sections from the passthrough so they no longer leak into
  generated check files, and keep 'name' pinned to the file basename.
- Reuse AMP_GLOBAL_DIR for AMP_CHECKS_GLOBAL_DIR.
- Add checksCount to the result-shape assertions in the MCP tests and
  add checks feature suites to lib/import and lib/convert tests.
- Update the DIR_FEATURES comment to mention checks.
- Document the tool-scoped section behavior for checks.

The reviewer's note about 'does not support the feature checks' warning
noise for wildcard features is intentionally unchanged: it is the
established warnUnsupportedTargets pattern shared by every feature core,
and changing warning policy is a cross-feature UX decision out of scope
here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: cm-dyoshikawa

docs/reference/file-formats.md

@@ -448,7 +448,7 @@ The emitted Amp frontmatter is derived from the source as follows:
 | `severity-default` | `severity`                                               |
 | `tools`            | `tools`                                                  |
 
-The frontmatter schema is loose, so any extra Amp-specific keys survive a generate/import round-trip. On import, `severity-default` maps back to the generic `severity` field, and the `name` field is dropped because it is re-derived from the file name on the next generate.
+The frontmatter schema is loose, so any extra Amp-specific keys survive a generate/import round-trip. A tool-scoped section (e.g. `amp: { "severity-default": "critical" }`) overrides the canonical values for that tool — the tool-specific value takes precedence, and the section itself is not emitted (except `name`, which always comes from the file name). On import, `severity-default` maps back to the generic `severity` field, and the `name` field is dropped because it is re-derived from the file name on the next generate.
 
 > **v1 limitation:** Amp also discovers subtree-scoped checks (e.g. `api/.agents/checks/`), but rulesync sources carry no directory-placement semantics, so those subtree-scoped checks are not generated. See the [Amp manual](https://ampcode.com/manual).
 

skills/rulesync/file-formats.md

@@ -448,7 +448,7 @@ The emitted Amp frontmatter is derived from the source as follows:
 | `severity-default` | `severity`                                               |
 | `tools`            | `tools`                                                  |
 
-The frontmatter schema is loose, so any extra Amp-specific keys survive a generate/import round-trip. On import, `severity-default` maps back to the generic `severity` field, and the `name` field is dropped because it is re-derived from the file name on the next generate.
+The frontmatter schema is loose, so any extra Amp-specific keys survive a generate/import round-trip. A tool-scoped section (e.g. `amp: { "severity-default": "critical" }`) overrides the canonical values for that tool — the tool-specific value takes precedence, and the section itself is not emitted (except `name`, which always comes from the file name). On import, `severity-default` maps back to the generic `severity` field, and the `name` field is dropped because it is re-derived from the file name on the next generate.
 
 > **v1 limitation:** Amp also discovers subtree-scoped checks (e.g. `api/.agents/checks/`), but rulesync sources carry no directory-placement semantics, so those subtree-scoped checks are not generated. See the [Amp manual](https://ampcode.com/manual).
 

src/cli/commands/gitignore-derive.ts

@@ -132,7 +132,7 @@ const deriveRulesEntries = (): GitignoreEntryTag[] => {
   return entries;
 };
 
-// commands/skills/subagents emit a directory tree; mcp/hooks/permissions/ignore
+// commands/skills/subagents/checks emit a directory tree; mcp/hooks/permissions/ignore
 // emit a single file; rules has a composite root+nonRoot shape.
 const DIR_FEATURES = new Set<Feature>(["commands", "skills", "subagents", "checks"]);
 const FILE_FEATURES = new Set<Feature>(["mcp", "hooks", "permissions", "ignore"]);

src/constants/amp-paths.ts

@@ -6,7 +6,7 @@ export const AMP_AGENTS_DIR = ".agents";
 export const AMP_SKILLS_PROJECT_DIR = join(AMP_AGENTS_DIR, "skills");
 export const AMP_SKILLS_GLOBAL_DIR = join(".config", "agents", "skills");
 export const AMP_CHECKS_PROJECT_DIR = join(AMP_AGENTS_DIR, "checks");
-export const AMP_CHECKS_GLOBAL_DIR = join(".config", "amp", "checks");
+export const AMP_CHECKS_GLOBAL_DIR = join(AMP_GLOBAL_DIR, "checks");
 export const AMP_RULE_FILE_NAME = "AGENTS.md";
 export const AMP_SETTINGS_FILE_NAME = "settings.json";
 export const AMP_SETTINGS_JSONC_FILE_NAME = "settings.jsonc";

src/features/checks/amp-check.test.ts

@@ -85,6 +85,36 @@ describe("AmpCheck.fromRulesyncCheck", () => {
     });
     expect(ampCheck.getRelativeDirPath()).toBe(AMP_CHECKS_GLOBAL_DIR);
   });
+
+  it("should let the amp-specific section win and not leak tool sections", () => {
+    const rulesyncCheck = new RulesyncCheck({
+      relativeDirPath: ".rulesync/checks",
+      relativeFilePath: "security.md",
+      frontmatter: {
+        targets: ["*"],
+        severity: "low",
+        custom: "kept",
+        amp: { "severity-default": "critical", name: "ignored" },
+        claudecode: { model: "opus" },
+      },
+      body: "Look for issues.",
+    });
+
+    const ampCheck = AmpCheck.fromRulesyncCheck({
+      rulesyncCheck,
+      relativeDirPath: ".rulesync/checks",
+    });
+    const frontmatter = ampCheck.getFrontmatter();
+
+    // The tool-specific value takes precedence over the canonical one.
+    expect(frontmatter["severity-default"]).toBe("critical");
+    // The check identity is the file basename; the section cannot rename it.
+    expect(frontmatter.name).toBe("security");
+    // Unknown non-tool keys pass through; tool sections themselves must not leak.
+    expect(frontmatter).toHaveProperty("custom", "kept");
+    expect(frontmatter).not.toHaveProperty("amp");
+    expect(frontmatter).not.toHaveProperty("claudecode");
+  });
 });
 
 describe("AmpCheck.toRulesyncCheck", () => {

src/features/checks/amp-check.ts

@@ -5,6 +5,7 @@ import { z } from "zod/mini";
 import { AMP_CHECKS_GLOBAL_DIR, AMP_CHECKS_PROJECT_DIR } from "../../constants/amp-paths.js";
 import { RULESYNC_CHECKS_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js";
 import { AiFileParams, ValidationResult } from "../../types/ai-file.js";
+import { ALL_TOOL_TARGETS } from "../../types/tool-targets.js";
 import { formatError } from "../../utils/error.js";
 import { readFileContent } from "../../utils/file.js";
 import { parseFrontmatter, stringifyFrontmatter } from "../../utils/frontmatter.js";
@@ -129,12 +130,24 @@ export class AmpCheck extends ToolCheck {
     // `severity-default`; everything else (including unknown keys) passes through.
     const { targets: _targets, description, severity, tools, ...restFields } = rulesyncFrontmatter;
 
+    // Tool-target sections (e.g. `amp:`) are per-tool overrides, not check
+    // content: exclude them all from the passthrough, then merge this tool's
+    // own section last so a tool-specific value takes precedence over the
+    // canonical one. `name` is excluded from the override because the check
+    // identity is carried by the file basename.
+    const toolTargetKeys = new Set<string>(ALL_TOOL_TARGETS);
+    const passthroughFields = Object.fromEntries(
+      Object.entries(restFields).filter(([key]) => !toolTargetKeys.has(key) && key !== "name"),
+    );
+    const ampSection = this.filterToolSpecificSection(rulesyncFrontmatter.amp ?? {}, ["name"]);
+
     const rawAmpFrontmatter = {
       name,
       ...(description !== undefined && { description }),
       ...(severity !== undefined && { "severity-default": severity }),
       ...(tools !== undefined && { tools }),
-      ...restFields,
+      ...passthroughFields,
+      ...ampSection,
     };
 
     const result = AmpCheckFrontmatterSchema.safeParse(rawAmpFrontmatter);

src/features/checks/tool-check.ts

@@ -76,4 +76,17 @@ export abstract class ToolCheck extends ToolFile {
 
     return false;
   }
+
+  protected static filterToolSpecificSection(
+    rawSection: Record<string, unknown>,
+    excludeFields: string[],
+  ): Record<string, unknown> {
+    const filtered: Record<string, unknown> = {};
+    for (const [key, value] of Object.entries(rawSection)) {
+      if (!excludeFields.includes(key)) {
+        filtered[key] = value;
+      }
+    }
+    return filtered;
+  }
 }

src/lib/convert.test.ts

@@ -1,5 +1,6 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
+import { ChecksProcessor } from "../features/checks/checks-processor.js";
 import { CommandsProcessor } from "../features/commands/commands-processor.js";
 import { HooksProcessor } from "../features/hooks/hooks-processor.js";
 import { IgnoreProcessor } from "../features/ignore/ignore-processor.js";
@@ -35,6 +36,7 @@ vi.mock("../features/commands/commands-processor.js");
 vi.mock("../features/skills/skills-processor.js");
 vi.mock("../features/hooks/hooks-processor.js");
 vi.mock("../features/permissions/permissions-processor.js");
+vi.mock("../features/checks/checks-processor.js");
 
 describe("convertFromTool", () => {
   let mockConfig: {
@@ -68,6 +70,7 @@ describe("convertFromTool", () => {
     vi.mocked(SkillsProcessor.getToolTargets).mockReturnValue(["claudecode", "cursor"]);
     vi.mocked(HooksProcessor.getToolTargets).mockReturnValue(["claudecode"]);
     vi.mocked(PermissionsProcessor.getToolTargets).mockReturnValue(["claudecode"]);
+    vi.mocked(ChecksProcessor.getToolTargets).mockReturnValue(["cursor", "claudecode"]);
 
     vi.mocked(RulesProcessor).mockImplementation(function () {
       return createMockProcessor() as unknown as RulesProcessor;
@@ -93,6 +96,9 @@ describe("convertFromTool", () => {
     vi.mocked(PermissionsProcessor).mockImplementation(function () {
       return createMockProcessor() as unknown as PermissionsProcessor;
     });
+    vi.mocked(ChecksProcessor).mockImplementation(function () {
+      return createMockProcessor() as unknown as ChecksProcessor;
+    });
   });
 
   afterEach(() => {
@@ -396,6 +402,45 @@ describe("convertFromTool", () => {
     });
   });
 
+  describe("checks feature", () => {
+    it("should convert check files", async () => {
+      mockConfig.getFeatures.mockReturnValue(["checks"]);
+
+      const result = await convertFromTool({
+        logger,
+        config: mockConfig as never,
+        fromTool: "cursor",
+        toTools: ["claudecode"],
+      });
+
+      expect(result.checksCount).toBe(1);
+    });
+
+    it("should return 0 when source has no check files", async () => {
+      mockConfig.getFeatures.mockReturnValue(["checks"]);
+
+      const mockProc = {
+        loadToolFiles: vi.fn().mockResolvedValue([]),
+        convertToolFilesToRulesyncFiles: vi.fn(),
+        convertRulesyncFilesToToolFiles: vi.fn(),
+        writeAiFiles: vi.fn(),
+      };
+      vi.mocked(ChecksProcessor).mockImplementation(function () {
+        return mockProc as unknown as ChecksProcessor;
+      });
+
+      const result = await convertFromTool({
+        logger,
+        config: mockConfig as never,
+        fromTool: "cursor",
+        toTools: ["claudecode"],
+      });
+
+      expect(result.checksCount).toBe(0);
+      expect(mockProc.convertToolFilesToRulesyncFiles).not.toHaveBeenCalled();
+    });
+  });
+
   describe("all features combined", () => {
     it("should return empty result when no features are enabled", async () => {
       mockConfig.getFeatures.mockReturnValue([]);
@@ -415,6 +460,7 @@ describe("convertFromTool", () => {
       expect(result.skillsCount).toBe(0);
       expect(result.hooksCount).toBe(0);
       expect(result.permissionsCount).toBe(0);
+      expect(result.checksCount).toBe(0);
     });
   });
 });

src/lib/import.test.ts

@@ -3,6 +3,7 @@ import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 import { RULESYNC_RELATIVE_DIR_PATH } from "../constants/rulesync-paths.js";
+import { ChecksProcessor } from "../features/checks/checks-processor.js";
 import { CommandsProcessor } from "../features/commands/commands-processor.js";
 import { HooksProcessor } from "../features/hooks/hooks-processor.js";
 import { IgnoreProcessor } from "../features/ignore/ignore-processor.js";
@@ -38,6 +39,7 @@ vi.mock("../features/commands/commands-processor.js");
 vi.mock("../features/skills/skills-processor.js");
 vi.mock("../features/hooks/hooks-processor.js");
 vi.mock("../features/permissions/permissions-processor.js");
+vi.mock("../features/checks/checks-processor.js");
 
 describe("importFromTool", () => {
   let mockConfig: {
@@ -69,6 +71,7 @@ describe("importFromTool", () => {
     vi.mocked(SkillsProcessor.getToolTargets).mockReturnValue(["claudecode"]);
     vi.mocked(HooksProcessor.getToolTargets).mockReturnValue(["claudecode"]);
     vi.mocked(PermissionsProcessor.getToolTargets).mockReturnValue(["claudecode"]);
+    vi.mocked(ChecksProcessor.getToolTargets).mockReturnValue(["claudecode"]);
 
     vi.mocked(RulesProcessor).mockImplementation(function () {
       return createMockProcessor() as unknown as RulesProcessor;
@@ -94,6 +97,9 @@ describe("importFromTool", () => {
     vi.mocked(PermissionsProcessor).mockImplementation(function () {
       return createMockProcessor() as unknown as PermissionsProcessor;
     });
+    vi.mocked(ChecksProcessor).mockImplementation(function () {
+      return createMockProcessor() as unknown as ChecksProcessor;
+    });
   });
 
   afterEach(() => {
@@ -760,6 +766,79 @@ describe("importFromTool", () => {
     });
   });
 
+  describe("checks feature", () => {
+    it("should import checks when feature is enabled", async () => {
+      mockConfig.getFeatures.mockReturnValue(["checks"]);
+
+      const result = await importFromTool({
+        logger,
+        config: mockConfig as never,
+        tool: "claudecode",
+      });
+
+      expect(result.checksCount).toBe(1);
+      expect(ChecksProcessor).toHaveBeenCalledWith(
+        expect.objectContaining({
+          outputRoot: ".",
+          toolTarget: "claudecode",
+          global: false,
+        }),
+      );
+    });
+
+    it("should return 0 checks when feature is not enabled", async () => {
+      mockConfig.getFeatures.mockReturnValue([]);
+
+      const result = await importFromTool({
+        logger,
+        config: mockConfig as never,
+        tool: "claudecode",
+      });
+
+      expect(result.checksCount).toBe(0);
+      expect(ChecksProcessor).not.toHaveBeenCalled();
+    });
+
+    it("should return 0 when tool is not supported", async () => {
+      mockConfig.getFeatures.mockReturnValue(["checks"]);
+      vi.mocked(ChecksProcessor.getToolTargets).mockReturnValue(["amp"]);
+
+      const result = await importFromTool({
+        logger,
+        config: mockConfig as never,
+        tool: "claudecode",
+      });
+
+      expect(result.checksCount).toBe(0);
+      expect(ChecksProcessor).not.toHaveBeenCalled();
+    });
+
+    it("should return 0 when no tool files found", async () => {
+      mockConfig.getFeatures.mockReturnValue(["checks"]);
+
+      const mockProcessor = {
+        loadToolFiles: vi.fn().mockResolvedValue([]),
+        convertToolFilesToRulesyncFiles: vi.fn(),
+        writeAiFiles: vi.fn(),
+      };
+      vi.mocked(ChecksProcessor).mockImplementation(function () {
+        return mockProcessor as unknown as ChecksProcessor;
+      });
+
+      const result = await importFromTool({
+        logger,
+        config: mockConfig as never,
+        tool: "claudecode",
+      });
+
+      expect(result.checksCount).toBe(0);
+      expect(mockProcessor.convertToolFilesToRulesyncFiles).not.toHaveBeenCalled();
+      expect(logger.warn).toHaveBeenCalledWith(
+        expect.stringContaining("No check files found for claudecode"),
+      );
+    });
+  });
+
   describe("all features combined", () => {
     it("should import all features when all are enabled", async () => {
       mockConfig.getFeatures.mockReturnValue([
@@ -770,6 +849,7 @@ describe("importFromTool", () => {
         "subagents",
         "skills",
         "permissions",
+        "checks",
       ]);
 
       const result = await importFromTool({
@@ -786,6 +866,7 @@ describe("importFromTool", () => {
       expect(result.skillsCount).toBe(1);
       expect(result.hooksCount).toBe(0);
       expect(result.permissionsCount).toBe(1);
+      expect(result.checksCount).toBe(1);
     });
 
     it("should return empty result when no features are enabled", async () => {
@@ -805,6 +886,7 @@ describe("importFromTool", () => {
       expect(result.skillsCount).toBe(0);
       expect(result.hooksCount).toBe(0);
       expect(result.permissionsCount).toBe(0);
+      expect(result.checksCount).toBe(0);
     });
   });
 

src/mcp/convert.test.ts

@@ -106,6 +106,7 @@ Body.
         skillsCount: expect.any(Number),
         hooksCount: expect.any(Number),
         permissionsCount: expect.any(Number),
+        checksCount: expect.any(Number),
         totalCount: expect.any(Number),
       });
     });