Improved rovodev support by nyanshak · Pull Request #1377 · dyoshikawa/rulesync

nyanshak · 2026-03-26T14:57:05Z

Fixes Rovodev support: code review findings from PR #1369 #1375 findings
Updates Rovodev feature support to better reflect reality.

Referenced docs:

Rovodev uses ruleDiscoveryMode "toon" like agentsmd and factorydroid but had no additionalConventions entry. Wire RovodevSubagent and RovodevSkill so synced AGENTS.md lists subagent and skill paths (no commands target). Clarify JSDoc: additional conventions embed paths for native features under toon discovery, not only simulated features.

typeof [] === "object", so parsed JSON could mis-classify an array as McpServers. Require a non-array object before tool MCP layers merge mcpServers. Add unit tests for the type guard.

…rn type Declare RovodevSubagent instead of ToolSubagent so call sites match RovodevSkill/RovodevMcp factory patterns and tests no longer need casts.

Stop spreading the full parsed Rovodev mcp.json into rulesync so unknown top-level keys (present or future) are not imported into .rulesync/mcp.json. Aligns with Roo/Deepagents; Cursor intentionally keeps extra keys. Add a regression test for ignored Rovodev-only top-level properties.

Extract parseRovodevMcpJson for constructor, fromFile, and fromRulesyncMcp so parse error messages stay consistent. Validate the parsed root value is a plain object via isRecord (arrays/scalars now fail with a clear error).

…agent.validate frontmatter is always assigned in the constructor, so the empty check never ran.

Align with Atlassian docs: project memory under .rovodev/AGENTS.md with a ./AGENTS.md mirror; localRoot -> AGENTS.local.md; global rules at ~/.rovodev/AGENTS.md. Import skills from .rovodev/skills and .agents/skills with primary-root precedence; emit skills to .rovodev/skills only; deletion enumerates every skill root. Document subagent dirs (.rovodev/subagents and ~/.rovodev/subagents). Mark Rovodev rules as supporting global mode in the supported-tools table and refresh file-formats localRoot notes.

dyoshikawa-claw · 2026-03-27T09:27:47Z

The main issues are:

gitignore-entries.ts was not updated. **/AGENTS.local.md, **/.rovodev/AGENTS.md, and **/.agents/skills/ should likely be ignored.
There is no guard in the nonRoot path handling for unexpected .md files being present.
AGENTS.local.md is supported for generation/deletion, but import support is not documented.
Converting AGENTS.local.md to a rulesync rule makes targets: ["*"], which feels a bit broad, so it would be good to confirm the regeneration impact.

Copilot

Pull request overview

Updates Rulesync’s Rovodev integration to better match current Rovo Dev CLI behavior and addresses the review findings tracked in #1375.

Changes:

Hardened MCP parsing/guards and reduced duplication; prevented Rovodev MCP tool-specific keys from leaking into Rulesync MCP.
Improved Rovodev rules behavior (global support, AGENTS.local.md support, mirroring .rovodev/AGENTS.md to ./AGENTS.md, deletion handling).
Added multi-root skill import support for Rovodev (.rovodev/skills primary, .agents/skills fallback) plus documentation updates.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/types/mcp.ts	Hardens `isMcpServers` to exclude arrays.
src/types/mcp.test.ts	Adds unit coverage for `isMcpServers`.
src/features/subagents/rovodev-subagent.ts	Improves Rovodev subagent docs; fixes return type; removes dead validation branch.
src/features/subagents/rovodev-subagent.test.ts	Removes now-unnecessary casts after return type fix.
src/features/skills/tool-skill.ts	Adds `alternativeSkillRoots` and helper for ordered root scanning.
src/features/skills/skills-processor.ts	Loads skills from multiple roots with precedence and dedupe; expands deletion scanning.
src/features/skills/skills-processor.test.ts	Tests Rovodev alternative root loading, precedence, and deletion listing.
src/features/skills/rovodev-skill.ts	Declares `.agents/skills` as an alternative import root; expands docs.
src/features/skills/rovodev-skill.test.ts	Tests settable paths and loading from `.agents/skills`.
src/features/rules/rules-processor.ts	Enables Rovodev global rules, adds additionalConventions metadata, adds AGENTS mirroring + AGENTS.local handling + deletion support.
src/features/rules/rules-processor.test.ts	Tests Rovodev global targets, AGENTS mirroring, and AGENTS.local deletion/generation behaviors.
src/features/rules/rovodev-rule.ts	Moves canonical root to `.rovodev/AGENTS.md`, adds project-root fallback, and maps `AGENTS.local.md` to `localRoot`.
src/features/rules/rovodev-rule.test.ts	Updates expectations for new Rovodev paths and localRoot mapping.
src/features/mcp/rovodev-mcp.ts	Centralizes JSON parsing/validation; ensures `toRulesyncMcp` only emits `mcpServers`.
src/features/mcp/rovodev-mcp.test.ts	Tests that unknown top-level keys don’t propagate into Rulesync MCP JSON.
skills/rulesync/supported-tools.md	Updates Rovodev support matrix to reflect global generation.
skills/rulesync/file-formats.md	Documents Rovodev `localRoot` output (`AGENTS.local.md`).
docs/reference/supported-tools.md	Mirrors supported-tools matrix update in docs.
docs/reference/file-formats.md	Mirrors localRoot documentation update in docs.
README.md	Updates supported tools table for Rovodev global generation.

Copilot · 2026-03-27T15:01:12Z

+      throw new Error(
+        `Rovodev AGENTS.md must be at ${join(paths.root.relativeDirPath, paths.root.relativeFilePath)} or project root, got: ${join(relativeDirPath, relativeFilePath)}`,


In fromFile(), the error message for disallowed locations always says "...or project root" even when global: true (where only .rovodev/AGENTS.md is allowed). This makes the thrown error misleading in global mode; consider branching the message based on whether alternativeRoots are available (project mode) vs global mode.

Suggested change

throw new Error(

`Rovodev AGENTS.md must be at ${join(paths.root.relativeDirPath, paths.root.relativeFilePath)} or project root, got: ${join(relativeDirPath, relativeFilePath)}`,

const expectedLocationDescription =

"alternativeRoots" in paths && paths.alternativeRoots && paths.alternativeRoots.length > 0

? `${join(paths.root.relativeDirPath, paths.root.relativeFilePath)} or project root`

: join(paths.root.relativeDirPath, paths.root.relativeFilePath);

throw new Error(

`Rovodev AGENTS.md must be at ${expectedLocationDescription}, got: ${join(relativeDirPath, relativeFilePath)}`,

Copilot · 2026-03-27T15:01:13Z

-      expect(paths.alternativeRoots).toBeUndefined();
+      expect(paths).toMatchObject({
+        alternativeRoots: [{ relativeDirPath: ".", relativeFilePath: "AGENTS.md" }],
+        nonRoot: { relativeDirPath: ".rovodev/.rulesync/modular-rules" },


This assertion hard-codes a POSIX-style path (.rovodev/.rulesync/modular-rules). Since the implementation uses join(), this can fail on Windows path separators. Use join(".rovodev", ".rulesync", "modular-rules") (or compare via toEqual with join) to keep the test OS-agnostic.

Suggested change

nonRoot: { relativeDirPath: ".rovodev/.rulesync/modular-rules" },

nonRoot: { relativeDirPath: RULESYNC_RULES_RELATIVE_DIR_PATH },

Copilot · 2026-03-27T15:01:13Z

+      nonRoot: {
+        relativeDirPath: join(".rovodev", ".rulesync", "modular-rules"),
      },


getSettablePaths() now declares a nonRoot directory, which makes RulesProcessor.loadToolFiles() scan that path for *.md. But RovodevRule.fromFile() currently throws for any relativeFilePath other than AGENTS.md, so the presence of any markdown file under this nonRoot directory would cause loadToolFiles() to error and return an empty result. Either omit nonRoot for Rovodev, or teach fromFile()/fromRulesyncRule() how to handle non-root rules under this directory.

Suggested change

nonRoot: {

relativeDirPath: join(".rovodev", ".rulesync", "modular-rules"),

},

Support markdown under .rovodev/.rulesync/modular-rules: fromFile when relativeDirPath matches modular root, fromRulesyncRule in project mode (with reserved AGENTS.md / AGENTS.local.md basenames rejected), and toRulesyncRule mapping with targets: [rovodev]. Let isTargetedByRulesyncRule include non-root rovodev-targeted rules so generate and simulated conventions work. RulesProcessor skips reserved modular paths on rovodev import with warnings, still lists all files for deletion, and passes relativeDirPath into non-root fromFile. Tighten AGENTS.md location errors so global mode does not mention project root. Map AGENTS.local.md import to targets: [rovodev]. Extend gitignore suggestions: **/AGENTS.local.md, **/.rovodev/AGENTS.md, **/.agents/skills/. Tests use join() for modular path expectations on Windows.

- Make DISALLOWED_ROVODEV_MODULAR_RULE_BASENAMES case-insensitive to prevent bypass on macOS/Windows filesystems (#7) - Scope AGENTS.local.md gitignore entry to rovodev target instead of common, since it is Rovodev-specific (#5) - Extract buildDeletionRulesFromPaths helper in rules-processor.ts to deduplicate the file-discovery-to-deletion pattern (#2) - Split RovodevRule.fromFile into fromModularFile/fromRootFile private helpers for improved readability (#4) - Add clarifying comment for nonRootPathsForImport unused on forDeletion path (#6) Closes #1392 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

refactor: address review findings from Rovodev PR #1377

nyanshak added 7 commits March 26, 2026 09:01

fix(mcp): reject arrays in isMcpServers guard

e9fab84

typeof [] === "object", so parsed JSON could mis-classify an array as McpServers. Require a non-array object before tool MCP layers merge mcpServers. Add unit tests for the type guard.

refactor(subagents): narrow RovodevSubagent.fromRulesyncSubagent retu…

0b7f79b

…rn type Declare RovodevSubagent instead of ToolSubagent so call sites match RovodevSkill/RovodevMcp factory patterns and tests no longer need casts.

refactor(mcp): centralize RovodevMcp JSON parsing

8fcfd65

Extract parseRovodevMcpJson for constructor, fromFile, and fromRulesyncMcp so parse error messages stay consistent. Validate the parsed root value is a plain object via isRecord (arrays/scalars now fail with a clear error).

refactor(subagents): drop unreachable frontmatter guard in RovodevSub…

bc3558b

…agent.validate frontmatter is always assigned in the constructor, so the empty check never ran.

This comment has been minimized.

Sign in to view

Repository owner deleted a comment from dyoshikawa-claw Mar 27, 2026

dyoshikawa requested a review from Copilot March 27, 2026 14:55

Copilot started reviewing on behalf of dyoshikawa March 27, 2026 14:55 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

dyoshikawa merged commit 926f3cd into dyoshikawa:main Mar 30, 2026
6 checks passed

dyoshikawa mentioned this pull request Mar 30, 2026

Rovodev support: review findings from PR #1377 #1392

Closed

Copilot AI mentioned this pull request Mar 30, 2026

refactor: address review findings from Rovodev PR #1377 #1396

Merged

3 tasks

dyoshikawa mentioned this pull request Mar 31, 2026

Rovodev rule: type guard extraction, closure testability, and minor cleanups #1402

Closed

dyoshikawa added a commit that referenced this pull request Mar 31, 2026

Merge pull request #1396 from dyoshikawa/issue-1392

8dc77fb

refactor: address review findings from Rovodev PR #1377

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improved rovodev support#1377

Improved rovodev support#1377
dyoshikawa merged 8 commits into
dyoshikawa:mainfrom
nyanshak:better-rovodev

nyanshak commented Mar 26, 2026

Uh oh!

This comment has been minimized.

dyoshikawa-claw commented Mar 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		throw new Error(
		`Rovodev AGENTS.md must be at ${join(paths.root.relativeDirPath, paths.root.relativeFilePath)} or project root, got: ${join(relativeDirPath, relativeFilePath)}`,

-      throw new Error(
-        `Rovodev AGENTS.md must be at ${join(paths.root.relativeDirPath, paths.root.relativeFilePath)} or project root, got: ${join(relativeDirPath, relativeFilePath)}`,
+      const expectedLocationDescription =
+        "alternativeRoots" in paths && paths.alternativeRoots && paths.alternativeRoots.length > 0
+          ? `${join(paths.root.relativeDirPath, paths.root.relativeFilePath)} or project root`
+          : join(paths.root.relativeDirPath, paths.root.relativeFilePath);
+      throw new Error(
+        `Rovodev AGENTS.md must be at ${expectedLocationDescription}, got: ${join(relativeDirPath, relativeFilePath)}`,

	nonRoot: { relativeDirPath: ".rovodev/.rulesync/modular-rules" },
	nonRoot: { relativeDirPath: RULESYNC_RULES_RELATIVE_DIR_PATH },

	nonRoot: {
	relativeDirPath: join(".rovodev", ".rulesync", "modular-rules"),
	},

Uh oh!

Conversation

nyanshak commented Mar 26, 2026

Uh oh!

This comment has been minimized.

dyoshikawa-claw commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dyoshikawa-claw commented Mar 27, 2026 •

edited

Loading