diff --git a/.gitignore b/.gitignore index 5d4b3ede0..6becb1f80 100644 --- a/.gitignore +++ b/.gitignore @@ -405,5 +405,6 @@ rulesync.local.jsonc **/.agents/checks/ **/.cursor/BUGBOT.md **/.hermes/plugins/rulesync-checks/checks/ +**/.rovodev/.review-agent.md !.rulesync/.aiignore # End of Rulesync diff --git a/README.md b/README.md index a38b3bf04..b89a9e8cd 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ The tables below show whether each tool supports a given feature (✅ = supporte | Kilo Code | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | | Kimi Code | ✅ | | ✅ | | ✅ | ✅ | ✅ | ✅ | | | Roo Code | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | | | -| Rovodev (Atlassian) | ✅ | | ✅ | ✅ | ✅ | ✅ | | ✅ | | +| Rovodev (Atlassian) | ✅ | | ✅ | ✅ | ✅ | ✅ | | ✅ | ✅ | | Takt | ✅ | | ✅ | ✅ | ✅ | ✅ | | ✅ | ✅ | | Vibe Code | ✅ | ✅ | ✅ | | ✅ | ✅ | ✅ | ✅ | | | Qwen Code | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index 22e198071..74e7e449b 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -516,7 +516,7 @@ Review the diff for injection vulnerabilities, hardcoded secrets, and unsafe deserialization. Report each finding with a file and line reference. ``` -Amp, Cursor, Hermes Agent and Takt consume checks. Amp receives one Markdown file per check: +Amp, Cursor, Hermes Agent, Rovo Dev CLI and Takt consume checks. Amp receives one Markdown file per check: - **Project scope:** `.agents/checks/.md` - **Global scope** (`--global`): `~/.config/amp/checks/.md` @@ -535,6 +535,8 @@ On import the markers split the file back into one check per section, each with Generating checks for Cursor replaces `.cursor/BUGBOT.md`, so run `rulesync import --targets cursor --features checks` first if the repository already has a hand-written one — generation warns when it is about to replace instructions rulesync did not write. Deletion is guarded: a `BUGBOT.md` holding anything rulesync did not write — no marker at all, or hand-written text ahead of the first marker — is never removed, so dropping the last check that targets Cursor takes rulesync's own output with it and nothing else. +For Rovo Dev CLI, checks are [code-review custom instructions](https://support.atlassian.com/rovo/docs/set-custom-instructions-for-code-reviews/), and Rovo Dev reads one plain-Markdown file rather than a file per check — so every check targeting Rovo Dev collapses into `.rovodev/.review-agent.md` (note the leading dot in the file name). The file takes **no frontmatter**. Everything else works exactly as it does for Cursor Bugbot above, because the two surfaces are the same shape: one marked-up section per check, `severity`/`tools` dropped, `description` used only when the body is empty, markers splitting the file back on import (with a hand-written file importing as a single `review-agent` check), the same `` escaping, the same replace-and-warn on generate, and the same deletion guard for a file holding anything rulesync did not write. Project scope only — these are per-repository review instructions and Rovo Dev documents no user-level equivalent, which is the opposite of the Rovo Dev permissions surface (global only). + For Hermes Agent, Rulesync writes project-local JSON specs under `.hermes/plugins/rulesync-checks/checks/` and a `rulesync-checks` plugin beside them. Its one-shot [`pre_verify` hook](https://hermes-agent.nousresearch.com/docs/user-guide/features/hooks/#pre-verify) fires only for coding turns with changed paths and `attempt == 0`, then asks Hermes to run all configured checks before finishing. `tools` is preserved as advisory guidance because Hermes does not enforce an Amp-style per-check tool allowlist. Run Hermes with the project plugin explicitly trusted for that invocation: ```sh @@ -1338,7 +1340,11 @@ For the Antigravity IDE, this generates `permissions.allow`, `permissions.ask`, For the Antigravity CLI (`agy`), this generates `permissions.allow`, `permissions.ask`, and `permissions.deny` arrays in the global `~/.gemini/antigravity-cli/settings.json` (**global mode only**). The CLI shares Antigravity 2.0's Fine-Grained Permissions Engine with the IDE, so the same `action(target)` vocabulary and `Deny > Ask > Allow` precedence apply: `read` → `read_file`, `edit`/`write` → `write_file`, `bash` → `command`, `webfetch`/`websearch` → `read_url`, `mcp` → `mcp` (the engine-only `execute_url` / `unsandboxed` actions pass through verbatim). Because `edit`/`write` collapse to `write_file` and `webfetch`/`websearch` collapse to `read_url`, importing normalizes back to `write` / `webfetch` (a documented, lossy mapping). The `settings.json` holds other CLI settings, so the `permissions` block is merged in place — entries for unmanaged actions are preserved — and the file is never deleted. Four CLI-only autonomy/sandbox knobs outside the allow/ask/deny arrays can be authored (and round-trip) through an optional `antigravity-cli` override block in `.rulesync/permissions.jsonc`: `toolPermission` (the global autonomy preset — `request-review` (default) / `proceed-in-sandbox` / `always-proceed` / `strict`), `enableTerminalSandbox` (a boolean confining agent-run commands to OS containment), `artifactReviewPolicy` (whether the agent's artifact changes are gated on a review prompt — `asks-for-review` (default) / `agent-decides` / `always-proceed`) and `allowNonWorkspaceAccess` (a boolean, off by default, letting the agent read or write files outside the active workspace roots). Antigravity applies the allow/deny lists as per-rule exceptions to the preset at runtime, so rulesync authors these keys verbatim as top-level siblings of `permissions` with no precedence modeling. This override is **CLI-only** — the Antigravity IDE exposes the same concepts through a GUI with no documented JSON schema, so it does not apply to `antigravity-ide`. Example: `{ "permission": { … }, "antigravity-cli": { "toolPermission": "strict", "enableTerminalSandbox": true, "artifactReviewPolicy": "agent-decides", "allowNonWorkspaceAccess": false } }`. Verified against the [Antigravity CLI reference](https://antigravity.google/docs/cli/reference), [sandbox docs](https://antigravity.google/docs/cli/sandbox) and [settings reference](https://antigravity.google/docs/cli/settings). See the [Antigravity CLI permissions docs](https://antigravity.google/docs/cli-permissions). -For Rovo Dev CLI, this generates the `toolPermissions` block of the global `~/.rovodev/config.yml` (**global mode only** — Rovo Dev has no project-scoped permissions file, mirroring the Rovodev MCP adapter). Rovo Dev's three levels (`allow`/`ask`/`deny`) are an exact 1:1 with rulesync's canonical actions, so action values pass through verbatim. The `bash` category maps the catch-all `*` pattern to `bash.default` and every other pattern to a `bash.commands[]` entry `{ command: , permission }` (Rovo Dev matches commands as regexes, so author `bash` patterns accordingly). The `read` category maps to the inspection tools (`open_files`, `expand_code_chunks`, `expand_folder`, `grep`) and `edit`/`write` to the mutation tools (`find_and_replace_code`, `create_file`, `delete_file`, `move_file`), written under **`toolPermissions.tools`** — the depth Rovo Dev documents. (Earlier Rulesync versions wrote them one level up, directly under `toolPermissions`, where Rovo Dev ignores them; import still reads that legacy shape as a fallback for keys the nested block says nothing about, so an old file is not lost, and a regenerate deletes the stale copies.) Because these per-tool keys hold a single level (no per-pattern rules), only the catch-all `*` of each category sets the level. Rovo Dev rewrites a single tool key when the user answers "always allow" to one prompt, so the four keys of a category can disagree; import collapses them back onto one catch-all by taking the strictest level (`deny` > `ask` > `allow`) rather than whichever key is read last. Tools outside these eight (Rovo Dev's Jira/Confluence and planning tools, for instance) have no canonical category, so a category catch-all does not reach them. Because `edit` and `write` both map onto the same mutation tools, a conflicting catch-all between them cannot be represented; the stricter of the two levels is kept — the same `deny` > `ask` > `allow` rule import uses — and a warning is logged. Non-catch-all `allow` paths in those categories are surfaced as `allowedExternalPaths` so explicit grants are not dropped; non-`allow` non-catch-all rules cannot be expressed per-path and are skipped with a warning. Categories without a clean Rovo Dev target (e.g. `webfetch`) are skipped with a warning. `config.yml` holds all of Rovo Dev's settings (`agent`, `sessions`, `mcp`, etc.), so the `toolPermissions` block is merged in place — every other top-level key is preserved, as is any key inside `toolPermissions` that Rulesync does not manage — including tools inside `toolPermissions.tools` that no canonical category maps to. The keys Rulesync does manage (`bash`, `allowedExternalPaths`, and the eight per-tool keys above) are owned rather than merged: each generate rewrites them from `.rulesync/permissions.*`, so removing a rule there removes it from `config.yml` too (a source stating no rule at all clears them; one whose rules simply have no Rovo Dev counterpart keeps the block's restrictions but strips its grants — an `allow` there is normally a leftover of an earlier generate, and dropping one falls back to Rovo Dev's stricter default, whereas clearing the whole block would relax every level), logging a warning naming each owned key it removes — per-tool levels and `allowedExternalPaths` are written from inside a Rovo Dev session too, by an "always allow" prompt answer and the `/directories` command, and a hand-edit to one of those keys — including a path added with the in-session `/directories` command, which writes to `allowedExternalPaths` — is replaced on the next generate (values only — YAML comments and formatting in the existing file are not retained on rewrite) — and the file is never deleted. See the [Rovo Dev CLI settings](https://support.atlassian.com/rovo/docs/manage-rovo-dev-cli-settings/) and [tool permissions](https://support.atlassian.com/rovo/docs/use-tools-in-rovo-dev-cli/) docs. +For Rovo Dev CLI, this generates the `toolPermissions` block of the global `~/.rovodev/config.yml` (**global mode only** — Rovo Dev has no project-scoped permissions file, mirroring the Rovodev MCP adapter). Rovo Dev's three levels (`allow`/`ask`/`deny`) are an exact 1:1 with rulesync's canonical actions, so action values pass through verbatim. The `bash` category maps the catch-all `*` pattern to `bash.default` and every other pattern to a `bash.commands[]` entry `{ command: , permission }` (Rovo Dev matches commands as regexes, so author `bash` patterns accordingly). The `read` category maps to the inspection tools (`open_files`, `expand_code_chunks`, `expand_folder`, `grep`) and `edit`/`write` to the mutation tools (`find_and_replace_code`, `create_file`, `delete_file`, `move_file`), written under **`toolPermissions.tools`** — the depth Rovo Dev documents. (Earlier Rulesync versions wrote them one level up, directly under `toolPermissions`, where Rovo Dev ignores them; import still reads that legacy shape as a fallback for keys the nested block says nothing about, so an old file is not lost, and a regenerate deletes the stale copies.) Because these per-tool keys hold a single level (no per-pattern rules), only the catch-all `*` of each category sets the level. Rovo Dev rewrites a single tool key when the user answers "always allow" to one prompt, so the four keys of a category can disagree; import collapses them back onto one catch-all by taking the strictest level (`deny` > `ask` > `allow`) rather than whichever key is read last. Rovo Dev's planning and Atlassian tools split the same way, so they ride the same two categories rather than getting one of their own: `read` also reaches `getJiraIssue` and `getConfluencePage`, and `edit`/`write` also reach `createJiraIssue`, `updateJiraIssue`, `createConfluencePage`, `updateConfluencePage` and `createTechnicalPlan` (grouped with the mutating tools because it is the planning tool that produces an artifact rather than reading one). Bear that in mind when authoring: an `edit: deny` reaches Jira and Confluence, not just the working tree. Because `edit` and `write` both map onto the same mutation tools, a conflicting catch-all between them cannot be represented; the stricter of the two levels is kept — the same `deny` > `ask` > `allow` rule import uses — and a warning is logged. Non-catch-all `allow` paths in those categories are surfaced as `allowedExternalPaths` so explicit grants are not dropped; non-`allow` non-catch-all rules cannot be expressed per-path and are skipped with a warning. Categories without a clean Rovo Dev target (e.g. `webfetch`) are skipped with a warning. `config.yml` holds all of Rovo Dev's settings (`agent`, `sessions`, `mcp`, etc.), so the `toolPermissions` block is merged in place — every other top-level key is preserved, as is any key inside `toolPermissions` that Rulesync does not manage — including tools inside `toolPermissions.tools` that no canonical category maps to. On **import**, a tool key the file is silent about counts as the implicit fallback level (`toolPermissions.default`, or Rovo Dev's own `ask`) rather than as absent, and the category still collapses to the strictest of the set. That matters because Rovo Dev writes a single key when the user answers "always allow" to one prompt: without the fallback, one such answer about `create_file` would import as a blanket `edit: allow`, and the next generate would hand that grant to every other tool of the category — Jira and Confluence writes included. A category the file says nothing about at all is still skipped rather than invented. + +**Migration note.** `toolPermissions.default` and the seven planning/Atlassian keys became Rulesync-owned in the release that added them. Ownership means the first generate after upgrading removes a hand-written value for one of them unless `.rulesync/permissions.*` produces it — a hand-written `tools.createJiraIssue: deny` or `default: deny` with no matching rule in the rulesync source is dropped (with a warning naming each key), falling back to Rovo Dev's `ask`. Run `rulesync import --targets rovodev --features permissions` before the first generate to carry those values into the rulesync source. + +The canonical all-tools category `*` maps to `toolPermissions.default`, the level Rovo Dev falls back to for any tool with no more specific setting (Rovo Dev's own default is `ask`) — derived from its catch-all exactly as `bash.default` is derived from `bash`'s, and round-tripped back on import. The default is a single level, so a pattern rule inside the `*` category has no counterpart and is skipped with a warning. The keys Rulesync does manage (`default`, `bash`, `allowedExternalPaths`, and the per-tool keys above) are owned rather than merged: each generate rewrites them from `.rulesync/permissions.*`, so removing a rule there removes it from `config.yml` too (a source stating no rule at all clears them; one whose rules simply have no Rovo Dev counterpart keeps the block's restrictions but strips its grants — an `allow` there is normally a leftover of an earlier generate, and dropping one falls back to Rovo Dev's stricter default, whereas clearing the whole block would relax every level), logging a warning naming each owned key it removes — per-tool levels and `allowedExternalPaths` are written from inside a Rovo Dev session too, by an "always allow" prompt answer and the `/directories` command, and a hand-edit to one of those keys — including a path added with the in-session `/directories` command, which writes to `allowedExternalPaths` — is replaced on the next generate (values only — YAML comments and formatting in the existing file are not retained on rewrite) — and the file is never deleted. See the [Rovo Dev CLI settings](https://support.atlassian.com/rovo/docs/manage-rovo-dev-cli-settings/) and [tool permissions](https://support.atlassian.com/rovo/docs/use-tools-in-rovo-dev-cli/) docs. For Goose, this generates the `user` block of the global `~/.config/goose/permission.yaml` (**global mode only** — Goose persists per-tool permission overrides only under the home directory and has no project-scoped permissions file). Goose stores permissions as a YAML map of mode key → `{ always_allow, ask_before, never_allow }`, where each field is a list of tool-name strings; rulesync writes the user-set decisions under the `user` key. Action mapping is a 1:1: `allow` → `always_allow`, `ask` → `ask_before`, `deny` → `never_allow`. Tool-name mapping: `bash` → `developer__shell`, `edit` → `developer__text_editor`; every other category passes through verbatim as the Goose tool name (so namespaced tools like `developer__text_editor` or `developer__image_processor` round-trip). Because Goose permission lists hold **whole tool names** rather than per-command/per-path globs, only a category's catch-all `*` pattern is representable — non-catch-all patterns are skipped with a warning. `write` collapses onto `developer__text_editor` too, so a conflicting `edit`/`write` catch-all cannot be represented; `edit` takes precedence and a warning is logged. The `permission.yaml` file is merged in place: the `user` block is owned by rulesync, while every other top-level key (notably the `smart_approve` LLM-decision cache) is preserved, and the file is never deleted. See the [Goose tool permissions docs](https://goose-docs.ai/docs/guides/managing-tools/tool-permissions/). diff --git a/docs/reference/supported-tools.md b/docs/reference/supported-tools.md index d835b74d4..bc313f4bc 100644 --- a/docs/reference/supported-tools.md +++ b/docs/reference/supported-tools.md @@ -25,7 +25,7 @@ Rulesync supports both **generation** and **import** for All of the major AI cod | Kilo Code | kilo | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | | Kimi Code | kimi-code | ✅ 🌏 | | ✅ 🌏 🔧 | | ✅ 🌏 | ✅ 🌏 | 🌏 | 🌏 | | | Roo Code | roo | ✅ 🌏 | ✅ | ✅ | ✅ 🌏 | ✅ | ✅ 🌏 | | | | -| Rovodev (Atlassian) | rovodev | ✅ 🌏 | | 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | 🌏 | | +| Rovodev (Atlassian) | rovodev | ✅ 🌏 | | 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | 🌏 | ✅ | | Takt | takt | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | | Vibe Code | vibe | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | | Qwen Code | qwencode | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index daaa0bb25..0095bdcd1 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -516,7 +516,7 @@ Review the diff for injection vulnerabilities, hardcoded secrets, and unsafe deserialization. Report each finding with a file and line reference. ``` -Amp, Cursor, Hermes Agent and Takt consume checks. Amp receives one Markdown file per check: +Amp, Cursor, Hermes Agent, Rovo Dev CLI and Takt consume checks. Amp receives one Markdown file per check: - **Project scope:** `.agents/checks/.md` - **Global scope** (`--global`): `~/.config/amp/checks/.md` @@ -535,6 +535,8 @@ On import the markers split the file back into one check per section, each with Generating checks for Cursor replaces `.cursor/BUGBOT.md`, so run `rulesync import --targets cursor --features checks` first if the repository already has a hand-written one — generation warns when it is about to replace instructions rulesync did not write. Deletion is guarded: a `BUGBOT.md` holding anything rulesync did not write — no marker at all, or hand-written text ahead of the first marker — is never removed, so dropping the last check that targets Cursor takes rulesync's own output with it and nothing else. +For Rovo Dev CLI, checks are [code-review custom instructions](https://support.atlassian.com/rovo/docs/set-custom-instructions-for-code-reviews/), and Rovo Dev reads one plain-Markdown file rather than a file per check — so every check targeting Rovo Dev collapses into `.rovodev/.review-agent.md` (note the leading dot in the file name). The file takes **no frontmatter**. Everything else works exactly as it does for Cursor Bugbot above, because the two surfaces are the same shape: one marked-up section per check, `severity`/`tools` dropped, `description` used only when the body is empty, markers splitting the file back on import (with a hand-written file importing as a single `review-agent` check), the same `` escaping, the same replace-and-warn on generate, and the same deletion guard for a file holding anything rulesync did not write. Project scope only — these are per-repository review instructions and Rovo Dev documents no user-level equivalent, which is the opposite of the Rovo Dev permissions surface (global only). + For Hermes Agent, Rulesync writes project-local JSON specs under `.hermes/plugins/rulesync-checks/checks/` and a `rulesync-checks` plugin beside them. Its one-shot [`pre_verify` hook](https://hermes-agent.nousresearch.com/docs/user-guide/features/hooks/#pre-verify) fires only for coding turns with changed paths and `attempt == 0`, then asks Hermes to run all configured checks before finishing. `tools` is preserved as advisory guidance because Hermes does not enforce an Amp-style per-check tool allowlist. Run Hermes with the project plugin explicitly trusted for that invocation: ```sh @@ -1338,7 +1340,11 @@ For the Antigravity IDE, this generates `permissions.allow`, `permissions.ask`, For the Antigravity CLI (`agy`), this generates `permissions.allow`, `permissions.ask`, and `permissions.deny` arrays in the global `~/.gemini/antigravity-cli/settings.json` (**global mode only**). The CLI shares Antigravity 2.0's Fine-Grained Permissions Engine with the IDE, so the same `action(target)` vocabulary and `Deny > Ask > Allow` precedence apply: `read` → `read_file`, `edit`/`write` → `write_file`, `bash` → `command`, `webfetch`/`websearch` → `read_url`, `mcp` → `mcp` (the engine-only `execute_url` / `unsandboxed` actions pass through verbatim). Because `edit`/`write` collapse to `write_file` and `webfetch`/`websearch` collapse to `read_url`, importing normalizes back to `write` / `webfetch` (a documented, lossy mapping). The `settings.json` holds other CLI settings, so the `permissions` block is merged in place — entries for unmanaged actions are preserved — and the file is never deleted. Four CLI-only autonomy/sandbox knobs outside the allow/ask/deny arrays can be authored (and round-trip) through an optional `antigravity-cli` override block in `.rulesync/permissions.jsonc`: `toolPermission` (the global autonomy preset — `request-review` (default) / `proceed-in-sandbox` / `always-proceed` / `strict`), `enableTerminalSandbox` (a boolean confining agent-run commands to OS containment), `artifactReviewPolicy` (whether the agent's artifact changes are gated on a review prompt — `asks-for-review` (default) / `agent-decides` / `always-proceed`) and `allowNonWorkspaceAccess` (a boolean, off by default, letting the agent read or write files outside the active workspace roots). Antigravity applies the allow/deny lists as per-rule exceptions to the preset at runtime, so rulesync authors these keys verbatim as top-level siblings of `permissions` with no precedence modeling. This override is **CLI-only** — the Antigravity IDE exposes the same concepts through a GUI with no documented JSON schema, so it does not apply to `antigravity-ide`. Example: `{ "permission": { … }, "antigravity-cli": { "toolPermission": "strict", "enableTerminalSandbox": true, "artifactReviewPolicy": "agent-decides", "allowNonWorkspaceAccess": false } }`. Verified against the [Antigravity CLI reference](https://antigravity.google/docs/cli/reference), [sandbox docs](https://antigravity.google/docs/cli/sandbox) and [settings reference](https://antigravity.google/docs/cli/settings). See the [Antigravity CLI permissions docs](https://antigravity.google/docs/cli-permissions). -For Rovo Dev CLI, this generates the `toolPermissions` block of the global `~/.rovodev/config.yml` (**global mode only** — Rovo Dev has no project-scoped permissions file, mirroring the Rovodev MCP adapter). Rovo Dev's three levels (`allow`/`ask`/`deny`) are an exact 1:1 with rulesync's canonical actions, so action values pass through verbatim. The `bash` category maps the catch-all `*` pattern to `bash.default` and every other pattern to a `bash.commands[]` entry `{ command: , permission }` (Rovo Dev matches commands as regexes, so author `bash` patterns accordingly). The `read` category maps to the inspection tools (`open_files`, `expand_code_chunks`, `expand_folder`, `grep`) and `edit`/`write` to the mutation tools (`find_and_replace_code`, `create_file`, `delete_file`, `move_file`), written under **`toolPermissions.tools`** — the depth Rovo Dev documents. (Earlier Rulesync versions wrote them one level up, directly under `toolPermissions`, where Rovo Dev ignores them; import still reads that legacy shape as a fallback for keys the nested block says nothing about, so an old file is not lost, and a regenerate deletes the stale copies.) Because these per-tool keys hold a single level (no per-pattern rules), only the catch-all `*` of each category sets the level. Rovo Dev rewrites a single tool key when the user answers "always allow" to one prompt, so the four keys of a category can disagree; import collapses them back onto one catch-all by taking the strictest level (`deny` > `ask` > `allow`) rather than whichever key is read last. Tools outside these eight (Rovo Dev's Jira/Confluence and planning tools, for instance) have no canonical category, so a category catch-all does not reach them. Because `edit` and `write` both map onto the same mutation tools, a conflicting catch-all between them cannot be represented; the stricter of the two levels is kept — the same `deny` > `ask` > `allow` rule import uses — and a warning is logged. Non-catch-all `allow` paths in those categories are surfaced as `allowedExternalPaths` so explicit grants are not dropped; non-`allow` non-catch-all rules cannot be expressed per-path and are skipped with a warning. Categories without a clean Rovo Dev target (e.g. `webfetch`) are skipped with a warning. `config.yml` holds all of Rovo Dev's settings (`agent`, `sessions`, `mcp`, etc.), so the `toolPermissions` block is merged in place — every other top-level key is preserved, as is any key inside `toolPermissions` that Rulesync does not manage — including tools inside `toolPermissions.tools` that no canonical category maps to. The keys Rulesync does manage (`bash`, `allowedExternalPaths`, and the eight per-tool keys above) are owned rather than merged: each generate rewrites them from `.rulesync/permissions.*`, so removing a rule there removes it from `config.yml` too (a source stating no rule at all clears them; one whose rules simply have no Rovo Dev counterpart keeps the block's restrictions but strips its grants — an `allow` there is normally a leftover of an earlier generate, and dropping one falls back to Rovo Dev's stricter default, whereas clearing the whole block would relax every level), logging a warning naming each owned key it removes — per-tool levels and `allowedExternalPaths` are written from inside a Rovo Dev session too, by an "always allow" prompt answer and the `/directories` command, and a hand-edit to one of those keys — including a path added with the in-session `/directories` command, which writes to `allowedExternalPaths` — is replaced on the next generate (values only — YAML comments and formatting in the existing file are not retained on rewrite) — and the file is never deleted. See the [Rovo Dev CLI settings](https://support.atlassian.com/rovo/docs/manage-rovo-dev-cli-settings/) and [tool permissions](https://support.atlassian.com/rovo/docs/use-tools-in-rovo-dev-cli/) docs. +For Rovo Dev CLI, this generates the `toolPermissions` block of the global `~/.rovodev/config.yml` (**global mode only** — Rovo Dev has no project-scoped permissions file, mirroring the Rovodev MCP adapter). Rovo Dev's three levels (`allow`/`ask`/`deny`) are an exact 1:1 with rulesync's canonical actions, so action values pass through verbatim. The `bash` category maps the catch-all `*` pattern to `bash.default` and every other pattern to a `bash.commands[]` entry `{ command: , permission }` (Rovo Dev matches commands as regexes, so author `bash` patterns accordingly). The `read` category maps to the inspection tools (`open_files`, `expand_code_chunks`, `expand_folder`, `grep`) and `edit`/`write` to the mutation tools (`find_and_replace_code`, `create_file`, `delete_file`, `move_file`), written under **`toolPermissions.tools`** — the depth Rovo Dev documents. (Earlier Rulesync versions wrote them one level up, directly under `toolPermissions`, where Rovo Dev ignores them; import still reads that legacy shape as a fallback for keys the nested block says nothing about, so an old file is not lost, and a regenerate deletes the stale copies.) Because these per-tool keys hold a single level (no per-pattern rules), only the catch-all `*` of each category sets the level. Rovo Dev rewrites a single tool key when the user answers "always allow" to one prompt, so the four keys of a category can disagree; import collapses them back onto one catch-all by taking the strictest level (`deny` > `ask` > `allow`) rather than whichever key is read last. Rovo Dev's planning and Atlassian tools split the same way, so they ride the same two categories rather than getting one of their own: `read` also reaches `getJiraIssue` and `getConfluencePage`, and `edit`/`write` also reach `createJiraIssue`, `updateJiraIssue`, `createConfluencePage`, `updateConfluencePage` and `createTechnicalPlan` (grouped with the mutating tools because it is the planning tool that produces an artifact rather than reading one). Bear that in mind when authoring: an `edit: deny` reaches Jira and Confluence, not just the working tree. Because `edit` and `write` both map onto the same mutation tools, a conflicting catch-all between them cannot be represented; the stricter of the two levels is kept — the same `deny` > `ask` > `allow` rule import uses — and a warning is logged. Non-catch-all `allow` paths in those categories are surfaced as `allowedExternalPaths` so explicit grants are not dropped; non-`allow` non-catch-all rules cannot be expressed per-path and are skipped with a warning. Categories without a clean Rovo Dev target (e.g. `webfetch`) are skipped with a warning. `config.yml` holds all of Rovo Dev's settings (`agent`, `sessions`, `mcp`, etc.), so the `toolPermissions` block is merged in place — every other top-level key is preserved, as is any key inside `toolPermissions` that Rulesync does not manage — including tools inside `toolPermissions.tools` that no canonical category maps to. On **import**, a tool key the file is silent about counts as the implicit fallback level (`toolPermissions.default`, or Rovo Dev's own `ask`) rather than as absent, and the category still collapses to the strictest of the set. That matters because Rovo Dev writes a single key when the user answers "always allow" to one prompt: without the fallback, one such answer about `create_file` would import as a blanket `edit: allow`, and the next generate would hand that grant to every other tool of the category — Jira and Confluence writes included. A category the file says nothing about at all is still skipped rather than invented. + +**Migration note.** `toolPermissions.default` and the seven planning/Atlassian keys became Rulesync-owned in the release that added them. Ownership means the first generate after upgrading removes a hand-written value for one of them unless `.rulesync/permissions.*` produces it — a hand-written `tools.createJiraIssue: deny` or `default: deny` with no matching rule in the rulesync source is dropped (with a warning naming each key), falling back to Rovo Dev's `ask`. Run `rulesync import --targets rovodev --features permissions` before the first generate to carry those values into the rulesync source. + +The canonical all-tools category `*` maps to `toolPermissions.default`, the level Rovo Dev falls back to for any tool with no more specific setting (Rovo Dev's own default is `ask`) — derived from its catch-all exactly as `bash.default` is derived from `bash`'s, and round-tripped back on import. The default is a single level, so a pattern rule inside the `*` category has no counterpart and is skipped with a warning. The keys Rulesync does manage (`default`, `bash`, `allowedExternalPaths`, and the per-tool keys above) are owned rather than merged: each generate rewrites them from `.rulesync/permissions.*`, so removing a rule there removes it from `config.yml` too (a source stating no rule at all clears them; one whose rules simply have no Rovo Dev counterpart keeps the block's restrictions but strips its grants — an `allow` there is normally a leftover of an earlier generate, and dropping one falls back to Rovo Dev's stricter default, whereas clearing the whole block would relax every level), logging a warning naming each owned key it removes — per-tool levels and `allowedExternalPaths` are written from inside a Rovo Dev session too, by an "always allow" prompt answer and the `/directories` command, and a hand-edit to one of those keys — including a path added with the in-session `/directories` command, which writes to `allowedExternalPaths` — is replaced on the next generate (values only — YAML comments and formatting in the existing file are not retained on rewrite) — and the file is never deleted. See the [Rovo Dev CLI settings](https://support.atlassian.com/rovo/docs/manage-rovo-dev-cli-settings/) and [tool permissions](https://support.atlassian.com/rovo/docs/use-tools-in-rovo-dev-cli/) docs. For Goose, this generates the `user` block of the global `~/.config/goose/permission.yaml` (**global mode only** — Goose persists per-tool permission overrides only under the home directory and has no project-scoped permissions file). Goose stores permissions as a YAML map of mode key → `{ always_allow, ask_before, never_allow }`, where each field is a list of tool-name strings; rulesync writes the user-set decisions under the `user` key. Action mapping is a 1:1: `allow` → `always_allow`, `ask` → `ask_before`, `deny` → `never_allow`. Tool-name mapping: `bash` → `developer__shell`, `edit` → `developer__text_editor`; every other category passes through verbatim as the Goose tool name (so namespaced tools like `developer__text_editor` or `developer__image_processor` round-trip). Because Goose permission lists hold **whole tool names** rather than per-command/per-path globs, only a category's catch-all `*` pattern is representable — non-catch-all patterns are skipped with a warning. `write` collapses onto `developer__text_editor` too, so a conflicting `edit`/`write` catch-all cannot be represented; `edit` takes precedence and a warning is logged. The `permission.yaml` file is merged in place: the `user` block is owned by rulesync, while every other top-level key (notably the `smart_approve` LLM-decision cache) is preserved, and the file is never deleted. See the [Goose tool permissions docs](https://goose-docs.ai/docs/guides/managing-tools/tool-permissions/). diff --git a/skills/rulesync/supported-tools.md b/skills/rulesync/supported-tools.md index d835b74d4..bc313f4bc 100644 --- a/skills/rulesync/supported-tools.md +++ b/skills/rulesync/supported-tools.md @@ -25,7 +25,7 @@ Rulesync supports both **generation** and **import** for All of the major AI cod | Kilo Code | kilo | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | | Kimi Code | kimi-code | ✅ 🌏 | | ✅ 🌏 🔧 | | ✅ 🌏 | ✅ 🌏 | 🌏 | 🌏 | | | Roo Code | roo | ✅ 🌏 | ✅ | ✅ | ✅ 🌏 | ✅ | ✅ 🌏 | | | | -| Rovodev (Atlassian) | rovodev | ✅ 🌏 | | 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | 🌏 | | +| Rovodev (Atlassian) | rovodev | ✅ 🌏 | | 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | 🌏 | ✅ | | Takt | takt | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | ✅ 🌏 | ✅ 🌏 | | Vibe Code | vibe | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | | Qwen Code | qwencode | ✅ 🌏 | ✅ | ✅ 🌏 🔧 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | | diff --git a/src/constants/rovodev-paths.ts b/src/constants/rovodev-paths.ts index 87351c86a..bfa386a37 100644 --- a/src/constants/rovodev-paths.ts +++ b/src/constants/rovodev-paths.ts @@ -11,3 +11,11 @@ export const ROVODEV_CONFIG_FILE_NAME = "config.yml"; export const ROVODEV_AGENTS_SKILLS_DIR_PATH = join(".agents", "skills"); export const ROVODEV_PROMPTS_FILE_NAME = "prompts.yml"; export const ROVODEV_PROMPTS_DIR_PATH = join(ROVODEV_DIR, "prompts"); + +/** + * Custom instructions for Rovo Dev's code reviews: a plain-Markdown file (no + * frontmatter) in the repository root's `.rovodev/` folder. Note the leading + * dot in the file name. + * @see https://support.atlassian.com/rovo/docs/set-custom-instructions-for-code-reviews/ + */ +export const ROVODEV_REVIEW_AGENT_FILE_NAME = ".review-agent.md"; diff --git a/src/e2e/e2e-checks.spec.ts b/src/e2e/e2e-checks.spec.ts index a5aafa64b..acbb6564b 100644 --- a/src/e2e/e2e-checks.spec.ts +++ b/src/e2e/e2e-checks.spec.ts @@ -27,6 +27,11 @@ const checksGenerateTargets = [ target: "hermesagent", outputPath: join(".hermes", "plugins", "rulesync-checks", "checks", "security.json"), }, + { + // Rovo Dev reads one plain-Markdown instruction file for code reviews. + target: "rovodev", + outputPath: join(".rovodev", ".review-agent.md"), + }, { // Takt's gates live in the shared config rather than in per-check files. target: "takt", @@ -88,11 +93,13 @@ Look for injection vulnerabilities. expect(generatedContent).toContain("Look for injection vulnerabilities."); return; } - if (target === "cursor") { + if (target === "cursor" || target === "rovodev") { // One marked-up section per check, keyed by the source file basename. expect(generatedContent).toContain(""); expect(generatedContent).toContain("## security"); expect(generatedContent).toContain("Look for injection vulnerabilities."); + // Plain prose: neither file takes frontmatter. + expect(generatedContent.startsWith("---")).toBe(false); return; } if (target === "amp") { @@ -118,6 +125,22 @@ Look for injection vulnerabilities. }, ); + it("should round-trip rovodev checks through import", async () => { + const testDir = getTestDir(); + + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + "Prefer small, well-named functions.\n", + ); + + await runImport({ target: "rovodev", features: "checks" }); + + const importedContent = await readFileContent( + join(testDir, RULESYNC_CHECKS_RELATIVE_DIR_PATH, "review-agent.md"), + ); + expect(importedContent).toContain("Prefer small, well-named functions."); + }); + it("should round-trip checks through import", async () => { const testDir = getTestDir(); diff --git a/src/features/checks/aggregated-check-file.test.ts b/src/features/checks/aggregated-check-file.test.ts new file mode 100644 index 000000000..d922a4150 --- /dev/null +++ b/src/features/checks/aggregated-check-file.test.ts @@ -0,0 +1,228 @@ +import { describe, expect, it } from "vitest"; + +import { RULESYNC_CHECKS_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js"; +import { + escapeCheckMarkers, + findCheckMarkers, + hasHandWrittenPreamble, + isOnlyGeneratedSections, + renderCheckFile, + renderCheckMarker, + splitCheckFile, + unescapeCheckMarkers, +} from "./aggregated-check-file.js"; +import { RulesyncCheck } from "./rulesync-check.js"; + +const checkOf = ({ + name, + body = "", + description, +}: { + name: string; + body?: string; + description?: string; +}): RulesyncCheck => + new RulesyncCheck({ + outputRoot: ".", + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + relativeFilePath: `${name}.md`, + frontmatter: { targets: ["*"], ...(description !== undefined && { description }) }, + body, + }); + +describe("aggregated-check-file", () => { + describe("findCheckMarkers", () => { + it("should find every marker and stay repeatable across calls", () => { + const content = [renderCheckMarker("a"), "body", renderCheckMarker("b")].join("\n"); + + expect(findCheckMarkers(content).map((marker) => marker.name)).toEqual(["a", "b"]); + // The pattern is module-level and global, so a second call must agree. + expect(findCheckMarkers(content).map((marker) => marker.name)).toEqual(["a", "b"]); + }); + + it("should ignore a marker that is not alone on its line", () => { + expect(findCheckMarkers(`text ${renderCheckMarker("a")}`)).toEqual([]); + }); + }); + + describe("hasHandWrittenPreamble", () => { + it.each([ + { label: "an empty file", content: "", expected: false }, + { label: "a whitespace-only file", content: "\n\n \n", expected: false }, + { label: "a file with no marker", content: "Hand-written.\n", expected: true }, + { + label: "text ahead of the first marker", + content: `Hand-written.\n${renderCheckMarker("a")}\n`, + expected: true, + }, + { + label: "only generated sections", + content: `${renderCheckMarker("a")}\n## a\n\nbody\n`, + expected: false, + }, + ])("should be $expected for $label", ({ content, expected }) => { + expect(hasHandWrittenPreamble(content)).toBe(expected); + }); + }); + + describe("isOnlyGeneratedSections", () => { + it.each([ + // Stricter than hasHandWrittenPreamble: with no marker there is nothing + // rulesync wrote, so an empty file is not rulesync's to delete either. + { label: "an empty file", content: "", expected: false }, + { label: "a whitespace-only file", content: "\n\n", expected: false }, + { label: "a file with no marker", content: "Hand-written.\n", expected: false }, + { + label: "text ahead of the first marker", + content: `Hand-written.\n${renderCheckMarker("a")}\n`, + expected: false, + }, + { + label: "only generated sections", + content: `${renderCheckMarker("a")}\n## a\n\nbody\n`, + expected: true, + }, + ])("should be $expected for $label", ({ content, expected }) => { + expect(isOnlyGeneratedSections(content)).toBe(expected); + }); + }); + + describe("marker escaping", () => { + it("should escape and unescape a marker line a body wrote itself", () => { + const body = renderCheckMarker("inner"); + const escaped = escapeCheckMarkers(body); + + expect(escaped).toBe(""); + expect(findCheckMarkers(escaped)).toEqual([]); + expect(unescapeCheckMarkers(escaped)).toBe(body); + }); + + it("should ladder so an already-escaped marker survives another round", () => { + const body = ""; + const escaped = escapeCheckMarkers(body); + + expect(escaped).toBe(""); + expect(unescapeCheckMarkers(escaped)).toBe(body); + }); + }); + + describe("renderCheckFile", () => { + it("should write a marker, a heading and the body per check", () => { + const content = renderCheckFile([checkOf({ name: "a", body: "Body A." })]); + + expect(content).toBe("\n## a\n\nBody A.\n"); + }); + + it("should fall back to the description when the body is empty", () => { + expect(renderCheckFile([checkOf({ name: "a", description: "Summary." })])).toContain( + "Summary.", + ); + }); + + it("should name a nested check by its basename only", () => { + const nested = new RulesyncCheck({ + outputRoot: ".", + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + relativeFilePath: "group/a.md", + frontmatter: { targets: ["*"] }, + body: "Body.", + }); + + expect(renderCheckFile([nested])).toContain(""); + }); + }); + + describe("splitCheckFile", () => { + it("should round-trip a rendered file", () => { + const checks = [checkOf({ name: "a", body: "Body A." }), checkOf({ name: "b", body: "B." })]; + + const split = splitCheckFile({ + fileContent: renderCheckFile(checks), + fallbackName: "fallback", + }); + + expect(split.map((check) => check.getRelativeFilePath())).toEqual(["a.md", "b.md"]); + expect(split.map((check) => check.getBody())).toEqual(["Body A.", "B."]); + }); + + it("should import a file with no marker as the fallback check", () => { + const split = splitCheckFile({ fileContent: "Prose.\n", fallbackName: "fallback" }); + + expect(split).toHaveLength(1); + expect(split[0]!.getRelativeFilePath()).toBe("fallback.md"); + expect(split[0]!.getBody()).toBe("Prose."); + }); + + it("should keep a preamble as its own check alongside the sections", () => { + const split = splitCheckFile({ + fileContent: `Preamble.\n\n${renderCheckMarker("a")}\n## a\n\nBody A.\n`, + fallbackName: "fallback", + }); + + expect(split.map((check) => check.getRelativeFilePath())).toEqual(["fallback.md", "a.md"]); + }); + + it("should return nothing for an empty file", () => { + expect(splitCheckFile({ fileContent: "", fallbackName: "fallback" })).toEqual([]); + }); + + it("should slugify a marker name so it cannot escape the checks directory", () => { + const split = splitCheckFile({ + fileContent: `${renderCheckMarker("../escape")}\nBody.\n`, + fallbackName: "fallback", + }); + + expect(split[0]!.getRelativeFilePath()).not.toContain(".."); + }); + + it("should suffix names that slugify the same so neither is lost", () => { + const split = splitCheckFile({ + fileContent: [ + `${renderCheckMarker("No Console")}`, + "first", + `${renderCheckMarker("no console")}`, + "second", + ].join("\n"), + fallbackName: "fallback", + }); + + expect(split).toHaveLength(2); + expect(split.map((check) => check.getRelativeFilePath())).toEqual([ + "no-console.md", + "no-console-2.md", + ]); + expect(split.map((check) => check.getBody())).toEqual(["first", "second"]); + }); + + it("should strip only the heading generate wrote, keeping a different one", () => { + const generated = splitCheckFile({ + fileContent: `${renderCheckMarker("a")}\n## a\n\nBody.\n`, + fallbackName: "fallback", + }); + const foreign = splitCheckFile({ + fileContent: `${renderCheckMarker("a")}\n## Something Else\n\nBody.\n`, + fallbackName: "fallback", + }); + + expect(generated[0]!.getBody()).toBe("Body."); + expect(foreign[0]!.getBody()).toBe("## Something Else\n\nBody."); + }); + + it("should match the heading against the raw marker name, not its slug", () => { + // A check named `No_Console` slugifies to `no-console`, but the heading + // generate wrote says `No_Console`. + const split = splitCheckFile({ + fileContent: `${renderCheckMarker("No_Console")}\n## No_Console\n\nBody.\n`, + fallbackName: "fallback", + }); + + expect(split[0]!.getBody()).toBe("Body."); + }); + + it("should import every check as applying to any tool", () => { + const split = splitCheckFile({ fileContent: "Prose.\n", fallbackName: "fallback" }); + + expect(split[0]!.getFrontmatter().targets).toEqual(["*"]); + }); + }); +}); diff --git a/src/features/checks/aggregated-check-file.ts b/src/features/checks/aggregated-check-file.ts new file mode 100644 index 000000000..84cbc7c5c --- /dev/null +++ b/src/features/checks/aggregated-check-file.ts @@ -0,0 +1,187 @@ +import { basename } from "node:path"; + +import { RULESYNC_CHECKS_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js"; +import { slugifyCheckName } from "./check-slug.js"; +import { RulesyncCheck } from "./rulesync-check.js"; + +/** + * Shared machinery for the tools whose checks surface is **one aggregated + * instruction file** rather than a file per check — Cursor Bugbot's + * `.cursor/BUGBOT.md` and Rovo Dev's `.rovodev/.review-agent.md`. + * + * Both read the file as free prose, so the check identities have to be carried + * in something invisible to the reader: an HTML-comment marker per section. + * That marker convention, the escaping that keeps a check body from splitting + * itself, and the import-side split are the same for both files, so they live + * here once. + */ + +/** Marks where one check starts inside the single instruction file. */ +const CHECK_MARKER_PATTERN = /^[ \t]*$/gm; + +/** + * A marker line a check body wrote itself — a rulesync doc fragment quoted in a + * code block, say. Emitting it verbatim would split that check in two on the + * next import, so `literal-` is inserted before `check:` on the way out and + * taken off on the way back. `(?:literal-)*` makes it a ladder, so a body that + * already contains an escaped marker survives the round trip too. + */ +const ESCAPABLE_MARKER_PATTERN = /^([ \t]*)$/gm; +const ESCAPED_MARKER_PATTERN = /^([ \t]*)$/gm; + +export function renderCheckMarker(name: string): string { + return ``; +} + +export function escapeCheckMarkers(content: string): string { + return content.replace(ESCAPABLE_MARKER_PATTERN, "$1literal-$2"); +} + +export function unescapeCheckMarkers(content: string): string { + return content.replace(ESCAPED_MARKER_PATTERN, "$1$2"); +} + +export type CheckMarker = { name: string; start: number; end: number }; + +export function findCheckMarkers(fileContent: string): CheckMarker[] { + // Reset lastIndex explicitly: the pattern is module-level and global. + CHECK_MARKER_PATTERN.lastIndex = 0; + const markers: CheckMarker[] = []; + let match: RegExpExecArray | null = CHECK_MARKER_PATTERN.exec(fileContent); + while (match !== null) { + markers.push({ + name: match[1] ?? "", + start: match.index, + end: match.index + match[0].length, + }); + match = CHECK_MARKER_PATTERN.exec(fileContent); + } + return markers; +} + +/** + * Whether the file holds instruction text ahead of the first marker — the + * question the "generating replaces this" warning asks. A file with no marker + * at all is entirely hand-written, so it qualifies; an empty one does not, + * since there is nothing to replace. + */ +export function hasHandWrittenPreamble(fileContent: string): boolean { + const firstMarkerStart = findCheckMarkers(fileContent)[0]?.start ?? fileContent.length; + return fileContent.slice(0, firstMarkerStart).trim().length > 0; +} + +/** + * Whether the file is nothing but sections rulesync generated — the question + * the deletion guard asks, and a stricter one than + * {@link hasHandWrittenPreamble}. A file carrying no marker at all is not + * rulesync's to remove even when it is empty: rulesync never wrote it, so an + * empty one is somebody's placeholder rather than our leftover. + */ +export function isOnlyGeneratedSections(fileContent: string): boolean { + const firstMarkerStart = findCheckMarkers(fileContent)[0]?.start; + if (firstMarkerStart === undefined) { + return false; + } + return fileContent.slice(0, firstMarkerStart).trim().length === 0; +} + +/** + * The instruction text one check contributes. Neither file has a field to put a + * summary in, so `description` is used only when there is no body. + */ +function toInstruction(rulesyncCheck: RulesyncCheck): string { + const body = rulesyncCheck.getBody().trim(); + if (body.length > 0) { + return body; + } + return rulesyncCheck.getFrontmatter().description?.trim() ?? ""; +} + +export function renderCheckSection(rulesyncCheck: RulesyncCheck): string { + // basename, so a check in a subdirectory does not name itself `dir/name`. + const name = basename(rulesyncCheck.getRelativeFilePath(), ".md"); + const heading = `## ${name}`; + const instruction = toInstruction(rulesyncCheck); + const lines = [renderCheckMarker(name), heading]; + if (instruction.length > 0) { + lines.push("", escapeCheckMarkers(instruction)); + } + return lines.join("\n"); +} + +export function renderCheckFile(rulesyncChecks: RulesyncCheck[]): string { + return `${rulesyncChecks.map(renderCheckSection).join("\n\n")}\n`; +} + +/** Drop the heading generate writes, so a round trip does not stack headings. */ +function stripGeneratedHeading(section: string, name: string): string { + const [firstLine, ...rest] = section.split("\n"); + if (firstLine?.trim() === `## ${name}`) { + return rest.join("\n").trim(); + } + return section.trim(); +} + +/** + * Split an aggregated instruction file back into one check per section. + * + * Content ahead of the first marker — and a hand-written file with no markers + * at all — becomes a single check named `fallbackName`, so nothing in the file + * is dropped. + */ +export function splitCheckFile({ + fileContent, + fallbackName, +}: { + fileContent: string; + fallbackName: string; +}): RulesyncCheck[] { + const sections: { name: string; content: string }[] = []; + const markers = findCheckMarkers(fileContent); + + const preambleEnd = markers[0]?.start ?? fileContent.length; + const preamble = fileContent.slice(0, preambleEnd).trim(); + if (preamble.length > 0) { + sections.push({ name: fallbackName, content: unescapeCheckMarkers(preamble) }); + } + + for (const [index, marker] of markers.entries()) { + const sectionEnd = markers[index + 1]?.start ?? fileContent.length; + const markerName = marker.name.trim(); + // The marker name comes from someone else's file on import, so it goes + // through the slug rules: a raw `../escape` would otherwise write outside + // the checks directory. + const name = slugifyCheckName(markerName) || fallbackName; + // Matched against the marker name rather than the slug, because that is + // what generate put in the heading — a check named `No_Console` would + // otherwise keep its heading and stack a second one on the next generate. + const content = stripGeneratedHeading( + fileContent.slice(marker.end, sectionEnd).trim(), + markerName, + ); + sections.push({ name, content: unescapeCheckMarkers(content) }); + } + + const used = new Set(); + return sections.map(({ name, content }) => { + // Two markers can slugify the same; a suffix keeps the second from + // overwriting the first. + let uniqueName = name; + let suffix = 2; + while (used.has(uniqueName)) { + uniqueName = `${name}-${suffix}`; + suffix += 1; + } + used.add(uniqueName); + + return new RulesyncCheck({ + outputRoot: ".", + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + relativeFilePath: `${uniqueName}.md`, + // These instructions are plain prose, so they apply to any tool — this + // imports the way an Amp check or a Takt string gate does. + frontmatter: { targets: ["*"] }, + body: content, + }); + }); +} diff --git a/src/features/checks/checks-processor.test.ts b/src/features/checks/checks-processor.test.ts index 6b5a22fe6..f0522a8bf 100644 --- a/src/features/checks/checks-processor.test.ts +++ b/src/features/checks/checks-processor.test.ts @@ -15,7 +15,13 @@ const logger = new ConsoleLogger({ verbose: false, silent: true }); describe("ChecksProcessor.getToolTargets", () => { it("should return project-scoped check targets", () => { - expect(ChecksProcessor.getToolTargets()).toEqual(["amp", "cursor", "hermesagent", "takt"]); + expect(ChecksProcessor.getToolTargets()).toEqual([ + "amp", + "cursor", + "hermesagent", + "rovodev", + "takt", + ]); }); it("should return amp for global scope", () => { diff --git a/src/features/checks/checks-processor.ts b/src/features/checks/checks-processor.ts index 2eb2cdfdb..9b7e41b1d 100644 --- a/src/features/checks/checks-processor.ts +++ b/src/features/checks/checks-processor.ts @@ -3,6 +3,7 @@ import { join, relative } from "node:path"; import { z } from "zod/mini"; import { CURSOR_BUGBOT_FILE_NAME } from "../../constants/cursor-paths.js"; +import { ROVODEV_REVIEW_AGENT_FILE_NAME } from "../../constants/rovodev-paths.js"; import { TAKT_CONFIG_FILE_NAME } from "../../constants/takt-paths.js"; import { FeatureProcessor } from "../../types/feature-processor.js"; import { RulesyncFile } from "../../types/rulesync-file.js"; @@ -15,6 +16,7 @@ import type { Logger } from "../../utils/logger.js"; import { AmpCheck } from "./amp-check.js"; import { CursorCheck } from "./cursor-check.js"; import { HermesagentCheck } from "./hermesagent-check.js"; +import { RovodevCheck } from "./rovodev-check.js"; import { RulesyncCheck } from "./rulesync-check.js"; import { TaktCheck } from "./takt-check.js"; import { @@ -95,6 +97,16 @@ export const toolCheckFactories = new Map[ \t]*$/gm; - -/** - * A marker line a check body wrote itself — a rulesync doc fragment quoted in a - * code block, say. Emitting it verbatim would split that check in two on the - * next import, so `literal-` is inserted before `check:` on the way out and - * taken off on the way back. `(?:literal-)*` makes it a ladder, so a body that - * already contains an escaped marker survives the round trip too. - */ -const ESCAPABLE_MARKER_PATTERN = /^([ \t]*)$/gm; -const ESCAPED_MARKER_PATTERN = /^([ \t]*)$/gm; - const FALLBACK_CHECK_NAME = "bugbot"; -function renderMarker(name: string): string { - return ``; -} - -function escapeMarkers(content: string): string { - return content.replace(ESCAPABLE_MARKER_PATTERN, "$1literal-$2"); -} - -function unescapeMarkers(content: string): string { - return content.replace(ESCAPED_MARKER_PATTERN, "$1$2"); -} - -type CheckMarker = { name: string; start: number; end: number }; - -function findMarkers(fileContent: string): CheckMarker[] { - // Reset lastIndex explicitly: the pattern is module-level and global. - CHECK_MARKER_PATTERN.lastIndex = 0; - const markers: CheckMarker[] = []; - let match: RegExpExecArray | null = CHECK_MARKER_PATTERN.exec(fileContent); - while (match !== null) { - markers.push({ - name: match[1] ?? "", - start: match.index, - end: match.index + match[0].length, - }); - match = CHECK_MARKER_PATTERN.exec(fileContent); - } - return markers; -} - -/** - * The instruction text one check contributes. Bugbot has no field to put a - * summary in, so `description` is used only when there is no body — the same - * fallback the file-stem heading above it gets. - */ -function toInstruction(rulesyncCheck: RulesyncCheck): string { - const body = rulesyncCheck.getBody().trim(); - if (body.length > 0) { - return body; - } - return rulesyncCheck.getFrontmatter().description?.trim() ?? ""; -} - -function renderSection(rulesyncCheck: RulesyncCheck): string { - // basename, so a check in a subdirectory does not name itself `dir/name`. - const name = basename(rulesyncCheck.getRelativeFilePath(), ".md"); - const heading = `## ${name}`; - const instruction = toInstruction(rulesyncCheck); - const lines = [renderMarker(name), heading]; - if (instruction.length > 0) { - lines.push("", escapeMarkers(instruction)); - } - return lines.join("\n"); -} - -/** Drop the heading generate writes, so a round trip does not stack headings. */ -function stripGeneratedHeading(section: string, name: string): string { - const [firstLine, ...rest] = section.split("\n"); - if (firstLine?.trim() === `## ${name}`) { - return rest.join("\n").trim(); - } - return section.trim(); -} - /** * Checks adapter for Cursor Bugbot (`.cursor/BUGBOT.md`). * @@ -161,11 +84,7 @@ export class CursorCheck extends ToolCheck { if (fileContent === null) { return true; } - const firstMarkerStart = findMarkers(fileContent)[0]?.start; - if (firstMarkerStart === undefined) { - return false; - } - return fileContent.slice(0, firstMarkerStart).trim().length === 0; + return isOnlyGeneratedSections(fileContent); } static override fromRulesyncCheck(_params: ToolCheckFromRulesyncCheckParams): CursorCheck { @@ -193,8 +112,7 @@ export class CursorCheck extends ToolCheck { // say so before hand-written instructions go away — the deletion guard // protects them, but generating over them cannot. const existingContent = (await readFileContentOrNull(filePath)) ?? ""; - const firstMarkerStart = findMarkers(existingContent)[0]?.start ?? existingContent.length; - if (existingContent.slice(0, firstMarkerStart).trim().length > 0) { + if (hasHandWrittenPreamble(existingContent)) { logger?.warn( `Cursor checks: ${filePath} holds instructions rulesync did not write, and generating ` + `replaces the whole file. Run \`rulesync import --targets cursor --features checks\` ` + @@ -202,7 +120,7 @@ export class CursorCheck extends ToolCheck { ); } - const fileContent = `${rulesyncChecks.map(renderSection).join("\n\n")}\n`; + const fileContent = renderCheckFile(rulesyncChecks); return [ new CursorCheck({ @@ -263,57 +181,9 @@ export class CursorCheck extends ToolCheck { } override toRulesyncChecks(): RulesyncCheck[] { - const fileContent = this.getFileContent(); - const sections: { name: string; content: string }[] = []; - const markers = findMarkers(fileContent); - - // Anything ahead of the first marker was hand-written beside the generated - // sections — or is the whole of a file rulesync never wrote. Either way it - // is instruction text, so it imports as its own check rather than vanishing. - const preambleEnd = markers[0]?.start ?? fileContent.length; - const preamble = fileContent.slice(0, preambleEnd).trim(); - if (preamble.length > 0) { - sections.push({ name: FALLBACK_CHECK_NAME, content: unescapeMarkers(preamble) }); - } - - for (const [index, marker] of markers.entries()) { - const sectionEnd = markers[index + 1]?.start ?? fileContent.length; - const markerName = marker.name.trim(); - // The marker name comes from someone else's BUGBOT.md on import, so it - // goes through the slug rules: a raw `../escape` would otherwise write - // outside the checks directory. - const name = slugifyCheckName(markerName) || FALLBACK_CHECK_NAME; - // Matched against the marker name rather than the slug, because that is - // what generate put in the heading — a check named `No_Console` would - // otherwise keep its heading and stack a second one on the next generate. - const content = stripGeneratedHeading( - fileContent.slice(marker.end, sectionEnd).trim(), - markerName, - ); - sections.push({ name, content: unescapeMarkers(content) }); - } - - const used = new Set(); - return sections.map(({ name, content }) => { - // Two markers can slugify the same; a suffix keeps the second from - // overwriting the first. - let uniqueName = name; - let suffix = 2; - while (used.has(uniqueName)) { - uniqueName = `${name}-${suffix}`; - suffix += 1; - } - used.add(uniqueName); - - return new RulesyncCheck({ - outputRoot: ".", - relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, - relativeFilePath: `${uniqueName}.md`, - // Bugbot instructions are plain prose, so they apply to any tool — this - // imports the way an Amp check or a Takt string gate does. - frontmatter: { targets: ["*"] }, - body: content, - }); + return splitCheckFile({ + fileContent: this.getFileContent(), + fallbackName: FALLBACK_CHECK_NAME, }); } } diff --git a/src/features/checks/rovodev-check.test.ts b/src/features/checks/rovodev-check.test.ts new file mode 100644 index 000000000..aff25879c --- /dev/null +++ b/src/features/checks/rovodev-check.test.ts @@ -0,0 +1,274 @@ +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { ROVODEV_REVIEW_AGENT_FILE_NAME } from "../../constants/rovodev-paths.js"; +import { RULESYNC_CHECKS_RELATIVE_DIR_PATH } from "../../constants/rulesync-paths.js"; +import { createMockLogger } from "../../test-utils/mock-logger.js"; +import { setupTestDirectory } from "../../test-utils/test-directories.js"; +import { ensureDir, writeFileContent } from "../../utils/file.js"; +import { RovodevCheck } from "./rovodev-check.js"; +import { RulesyncCheck } from "./rulesync-check.js"; + +const checkOf = ({ + name, + body, + description, +}: { + name: string; + body: string; + description?: string; +}): RulesyncCheck => + new RulesyncCheck({ + outputRoot: ".", + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + relativeFilePath: `${name}.md`, + frontmatter: { targets: ["*"], ...(description !== undefined && { description }) }, + body, + }); + +describe("RovodevCheck", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + vi.spyOn(process, "cwd").mockReturnValue(testDir); + }); + + afterEach(async () => { + await cleanup(); + vi.restoreAllMocks(); + }); + + describe("getSettablePaths", () => { + it("should point at the dotfile in the .rovodev directory", () => { + expect(RovodevCheck.getSettablePaths()).toEqual({ + relativeDirPath: ".rovodev", + relativeFilePath: ".review-agent.md", + }); + }); + }); + + describe("fromRulesyncChecks", () => { + it("should aggregate every check into one frontmatter-free file", async () => { + const [check] = await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [ + checkOf({ name: "no-console", body: "Flag console.log calls." }), + checkOf({ name: "naming", body: "Enforce kebab-case file names." }), + ], + }); + + const content = check!.getFileContent(); + expect(content).not.toMatch(/^---/); + expect(content).toContain(""); + expect(content).toContain("## no-console"); + expect(content).toContain("Flag console.log calls."); + expect(content).toContain("Enforce kebab-case file names."); + expect(check!.getRelativeFilePath()).toBe(".review-agent.md"); + }); + + it("should fall back to the description when a check has no body", async () => { + const [check] = await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [checkOf({ name: "typing", body: "", description: "No any." })], + }); + + expect(check!.getFileContent()).toContain("No any."); + }); + + it("should write nothing when no check targets Rovo Dev", async () => { + const checks = await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [], + }); + + expect(checks).toEqual([]); + }); + + it("should warn before replacing hand-written instructions", async () => { + await ensureDir(join(testDir, ".rovodev")); + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + "Hand-written review notes.\n", + ); + const logger = createMockLogger(); + + await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [checkOf({ name: "no-console", body: "Flag console.log calls." })], + logger, + }); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("rulesync did not write")); + }); + + it("should stay quiet when the existing file is only generated sections", async () => { + const [generated] = await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [checkOf({ name: "no-console", body: "Flag console.log calls." })], + }); + await ensureDir(join(testDir, ".rovodev")); + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + generated!.getFileContent(), + ); + const logger = createMockLogger(); + + await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [checkOf({ name: "no-console", body: "Flag console.log calls." })], + logger, + }); + + expect(logger.warn).not.toHaveBeenCalled(); + }); + }); + + describe("canDeleteAuxiliaryFiles", () => { + it("should allow deletion when the file does not exist", async () => { + expect(await RovodevCheck.canDeleteAuxiliaryFiles({ outputRoot: testDir })).toBe(true); + }); + + it("should refuse deletion when the file holds hand-written instructions", async () => { + await ensureDir(join(testDir, ".rovodev")); + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + "Hand-written review notes.\n", + ); + + expect(await RovodevCheck.canDeleteAuxiliaryFiles({ outputRoot: testDir })).toBe(false); + }); + + it("should allow deletion when the file is only generated sections", async () => { + const [generated] = await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [checkOf({ name: "no-console", body: "Flag console.log calls." })], + }); + await ensureDir(join(testDir, ".rovodev")); + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + generated!.getFileContent(), + ); + + expect(await RovodevCheck.canDeleteAuxiliaryFiles({ outputRoot: testDir })).toBe(true); + }); + }); + + describe("toRulesyncChecks", () => { + it("should split generated sections back into one check each", async () => { + const [generated] = await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [ + checkOf({ name: "no-console", body: "Flag console.log calls." }), + checkOf({ name: "naming", body: "Enforce kebab-case file names." }), + ], + }); + await ensureDir(join(testDir, ".rovodev")); + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + generated!.getFileContent(), + ); + + const imported = ( + await RovodevCheck.fromFile({ + outputRoot: testDir, + relativeFilePath: ROVODEV_REVIEW_AGENT_FILE_NAME, + }) + ).toRulesyncChecks(); + + expect(imported.map((check) => check.getRelativeFilePath())).toEqual([ + "no-console.md", + "naming.md", + ]); + expect(imported[0]!.getBody()).toBe("Flag console.log calls."); + expect(imported[0]!.getFrontmatter().targets).toEqual(["*"]); + }); + + it("should import a hand-written file as a single review-agent check", async () => { + await ensureDir(join(testDir, ".rovodev")); + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + "Prefer small functions.\n", + ); + + const imported = ( + await RovodevCheck.fromFile({ + outputRoot: testDir, + relativeFilePath: ROVODEV_REVIEW_AGENT_FILE_NAME, + }) + ).toRulesyncChecks(); + + expect(imported).toHaveLength(1); + expect(imported[0]!.getRelativeFilePath()).toBe("review-agent.md"); + expect(imported[0]!.getBody()).toBe("Prefer small functions."); + }); + + it("should round-trip a body that contains a marker line", async () => { + const body = "Example:\n\n"; + const [generated] = await RovodevCheck.fromRulesyncChecks({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncChecks: [checkOf({ name: "docs", body })], + }); + await ensureDir(join(testDir, ".rovodev")); + await writeFileContent( + join(testDir, ".rovodev", ".review-agent.md"), + generated!.getFileContent(), + ); + + const imported = ( + await RovodevCheck.fromFile({ + outputRoot: testDir, + relativeFilePath: ROVODEV_REVIEW_AGENT_FILE_NAME, + }) + ).toRulesyncChecks(); + + expect(imported).toHaveLength(1); + expect(imported[0]!.getBody()).toBe(body); + }); + }); + + describe("isTargetedByRulesyncCheck", () => { + it("should respect the targets list", () => { + const targeted = new RulesyncCheck({ + outputRoot: ".", + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + relativeFilePath: "a.md", + frontmatter: { targets: ["rovodev"] }, + body: "b", + }); + const notTargeted = new RulesyncCheck({ + outputRoot: ".", + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + relativeFilePath: "b.md", + frontmatter: { targets: ["cursor"] }, + body: "b", + }); + + expect(RovodevCheck.isTargetedByRulesyncCheck(targeted)).toBe(true); + expect(RovodevCheck.isTargetedByRulesyncCheck(notTargeted)).toBe(false); + }); + }); + + describe("fromRulesyncCheck", () => { + it("should refuse per-check conversion", () => { + expect(() => + RovodevCheck.fromRulesyncCheck({ + outputRoot: testDir, + relativeDirPath: RULESYNC_CHECKS_RELATIVE_DIR_PATH, + rulesyncCheck: checkOf({ name: "a", body: "b" }), + }), + ).toThrow(/fromRulesyncChecks/); + }); + }); +}); diff --git a/src/features/checks/rovodev-check.ts b/src/features/checks/rovodev-check.ts new file mode 100644 index 000000000..97408ab23 --- /dev/null +++ b/src/features/checks/rovodev-check.ts @@ -0,0 +1,176 @@ +import { join } from "node:path"; + +import { ROVODEV_DIR, ROVODEV_REVIEW_AGENT_FILE_NAME } from "../../constants/rovodev-paths.js"; +import type { ValidationResult } from "../../types/ai-file.js"; +import { readFileContentOrNull } from "../../utils/file.js"; +import { + hasHandWrittenPreamble, + isOnlyGeneratedSections, + renderCheckFile, + splitCheckFile, +} from "./aggregated-check-file.js"; +import { RulesyncCheck } from "./rulesync-check.js"; +import { + ToolCheck, + type ToolCheckForDeletionParams, + type ToolCheckFromFileParams, + type ToolCheckFromRulesyncCheckParams, + type ToolCheckFromRulesyncChecksParams, + type ToolCheckSettablePaths, +} from "./tool-check.js"; + +const FALLBACK_CHECK_NAME = "review-agent"; + +/** + * Checks adapter for Rovo Dev CLI's code-review custom instructions + * (`.rovodev/.review-agent.md`). + * + * Rovo Dev takes one plain-Markdown instruction file at the repository root's + * `.rovodev/` folder — no frontmatter, and note the leading dot in the file + * name. Like Cursor Bugbot it is a single aggregated file rather than a file + * per check, so every `.rulesync/checks/*.md` targeting Rovo Dev collapses into + * it via {@link fromRulesyncChecks}, with each check written as a marked + * section (see `aggregated-check-file.ts` for the marker convention the two + * adapters share). + * + * Rovo Dev reads the file as free prose, so a check's `severity` and `tools` + * have no equivalent there: they are not written and do not come back on + * import. Neither does `description` whenever the check also has a body. + * + * Project scope only — these are per-repository review instructions, and Rovo + * Dev documents no user-level equivalent. (The `permissions` adapter for the + * same tool is the opposite: global only.) + * + * @see https://support.atlassian.com/rovo/docs/set-custom-instructions-for-code-reviews/ + */ +export class RovodevCheck extends ToolCheck { + static getSettablePaths(_options: { global?: boolean } = {}): ToolCheckSettablePaths { + // Naming the file keeps consumers that would otherwise claim the whole + // `.rovodev/` tree — the gitignore derivation, for one — narrowed to the + // one file written, since every other Rovo Dev feature writes there too. + return { relativeDirPath: ROVODEV_DIR, relativeFilePath: ROVODEV_REVIEW_AGENT_FILE_NAME }; + } + + static isTargetedByRulesyncCheck(rulesyncCheck: RulesyncCheck): boolean { + return this.isTargetedByRulesyncCheckDefault({ rulesyncCheck, toolTarget: "rovodev" }); + } + + /** + * Ownership guard the processor consults before it deletes anything for this + * tool. `.review-agent.md` is a file Rovo Dev's own documentation tells users + * to hand-write, so anything in it that rulesync did not write is not + * rulesync's to remove — dropping the last check targeting Rovo Dev must not + * take somebody's hand-written review instructions with it. + */ + static async canDeleteAuxiliaryFiles({ outputRoot }: { outputRoot: string }): Promise { + const paths = RovodevCheck.getSettablePaths(); + const filePath = join( + outputRoot, + paths.relativeDirPath, + paths.relativeFilePath ?? ROVODEV_REVIEW_AGENT_FILE_NAME, + ); + const fileContent = await readFileContentOrNull(filePath); + if (fileContent === null) { + return true; + } + return isOnlyGeneratedSections(fileContent); + } + + static override fromRulesyncCheck(_params: ToolCheckFromRulesyncCheckParams): RovodevCheck { + // Sections share one file, so they are only ever built as a set. + throw new Error("Rovo Dev checks are built from all checks at once; use fromRulesyncChecks."); + } + + static async fromRulesyncChecks({ + outputRoot = process.cwd(), + rulesyncChecks, + global = false, + logger, + }: ToolCheckFromRulesyncChecksParams): Promise { + if (rulesyncChecks.length === 0) { + // No section to write. A stale file from an earlier generate is removed by + // the processor's deletion pass rather than by an empty file written here. + return []; + } + + const paths = RovodevCheck.getSettablePaths({ global }); + const relativeFilePath = paths.relativeFilePath ?? ROVODEV_REVIEW_AGENT_FILE_NAME; + const filePath = join(outputRoot, paths.relativeDirPath, relativeFilePath); + + // The file is rewritten from `.rulesync/checks/` rather than merged into, so + // say so before hand-written instructions go away — the deletion guard + // protects them, but generating over them cannot. + const existingContent = (await readFileContentOrNull(filePath)) ?? ""; + if (hasHandWrittenPreamble(existingContent)) { + logger?.warn( + `Rovo Dev checks: ${filePath} holds instructions rulesync did not write, and generating ` + + `replaces the whole file. Run \`rulesync import --targets rovodev --features checks\` ` + + `first to keep them.`, + ); + } + + return [ + new RovodevCheck({ + outputRoot, + relativeDirPath: paths.relativeDirPath, + relativeFilePath, + fileContent: renderCheckFile(rulesyncChecks), + global, + }), + ]; + } + + static async fromFile({ + outputRoot = process.cwd(), + global = false, + }: ToolCheckFromFileParams): Promise { + const paths = RovodevCheck.getSettablePaths({ global }); + const relativeFilePath = paths.relativeFilePath ?? ROVODEV_REVIEW_AGENT_FILE_NAME; + const filePath = join(outputRoot, paths.relativeDirPath, relativeFilePath); + return new RovodevCheck({ + outputRoot, + relativeDirPath: paths.relativeDirPath, + relativeFilePath, + fileContent: (await readFileContentOrNull(filePath)) ?? "", + global, + }); + } + + static forDeletion({ + outputRoot = process.cwd(), + relativeDirPath, + relativeFilePath, + global = false, + }: ToolCheckForDeletionParams): RovodevCheck { + return new RovodevCheck({ + outputRoot, + relativeDirPath, + relativeFilePath, + fileContent: "", + validate: false, + global, + }); + } + + validate(): ValidationResult { + return { success: true, error: null }; + } + + toRulesyncCheck(): RulesyncCheck { + const checks = this.toRulesyncChecks(); + const first = checks[0]; + if (!first) { + throw new Error( + `No check instructions found in ${join(this.getRelativeDirPath(), this.getRelativeFilePath())}.`, + ); + } + return first; + } + + override toRulesyncChecks(): RulesyncCheck[] { + return splitCheckFile({ + fileContent: this.getFileContent(), + fallbackName: FALLBACK_CHECK_NAME, + }); + } +} diff --git a/src/features/permissions/rovodev-permissions.test.ts b/src/features/permissions/rovodev-permissions.test.ts index 47d3df0a1..bc95607c2 100644 --- a/src/features/permissions/rovodev-permissions.test.ts +++ b/src/features/permissions/rovodev-permissions.test.ts @@ -282,6 +282,12 @@ describe("RovodevPermissions", () => { open_files: "allow", expand_folder: "allow", expand_code_chunks: "allow", + // Every key of the `read` category has to say `allow` for the + // catch-all to be `allow`; a silent one counts as the implicit + // level. Set them here so this test isolates the nested-vs-legacy + // precedence question rather than re-testing that fallback. + getJiraIssue: "allow", + getConfluencePage: "allow", tools: { grep: "allow" }, }, }), @@ -596,7 +602,7 @@ describe("RovodevPermissions", () => { await ensureDir(dirPath); await writeFileContent( join(dirPath, "config.yml"), - dump({ toolPermissions: { tools: { createTechnicalPlan: "allow" } } }), + dump({ toolPermissions: { tools: { someFutureRovodevTool: "allow" } } }), ); const perms = await RovodevPermissions.fromRulesyncPermissions({ @@ -606,8 +612,185 @@ describe("RovodevPermissions", () => { }); const tools = toolLevelsOf(perms.getFileContent()); - expect(tools.createTechnicalPlan).toBe("allow"); + expect(tools.someFutureRovodevTool).toBe("allow"); expect(tools.grep).toBe("deny"); }); }); + + describe("planning and Atlassian tool keys", () => { + it("maps the read category onto the inspection tools of both surfaces", async () => { + const perms = await RovodevPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions: rulesyncPermissions({ read: { "*": "deny" } }), + global: true, + }); + + const tools = toolLevelsOf(perms.getFileContent()); + expect(tools.grep).toBe("deny"); + expect(tools.getJiraIssue).toBe("deny"); + expect(tools.getConfluencePage).toBe("deny"); + }); + + it("maps the edit category onto the mutating tools of both surfaces", async () => { + const perms = await RovodevPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions: rulesyncPermissions({ edit: { "*": "deny" } }), + global: true, + }); + + const tools = toolLevelsOf(perms.getFileContent()); + expect(tools.create_file).toBe("deny"); + expect(tools.createTechnicalPlan).toBe("deny"); + expect(tools.createJiraIssue).toBe("deny"); + expect(tools.updateJiraIssue).toBe("deny"); + expect(tools.createConfluencePage).toBe("deny"); + expect(tools.updateConfluencePage).toBe("deny"); + }); + + it("imports the new keys back into their canonical categories", async () => { + const dirPath = join(testDir, ".rovodev"); + await ensureDir(dirPath); + await writeFileContent( + join(dirPath, "config.yml"), + dump({ + toolPermissions: { + tools: { getJiraIssue: "deny", createConfluencePage: "ask" }, + }, + }), + ); + + const perms = await RovodevPermissions.fromFile({ outputRoot: testDir, global: true }); + const imported = JSON.parse(perms.toRulesyncPermissions().getFileContent()); + + expect(imported.permission.read["*"]).toBe("deny"); + expect(imported.permission.edit["*"]).toBe("ask"); + }); + }); + + describe("toolPermissions.default", () => { + it("maps the all-tools catch-all onto the tool-wide default", async () => { + const perms = await RovodevPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions: rulesyncPermissions({ + "*": { "*": "deny" }, + bash: { "*": "ask" }, + }), + global: true, + }); + + const parsed = load(perms.getFileContent()) as { + toolPermissions: { default?: string; bash?: { default?: string } }; + }; + expect(parsed.toolPermissions.default).toBe("deny"); + // The two defaults are derived the same way and stay independent. + expect(parsed.toolPermissions.bash?.default).toBe("ask"); + }); + + it("warns and skips a pattern rule in the all-tools category", async () => { + const logger = createMockLogger(); + + const perms = await RovodevPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions: rulesyncPermissions({ "*": { "src/**": "deny", "*": "ask" } }), + global: true, + logger, + }); + + const parsed = load(perms.getFileContent()) as { + toolPermissions: { default?: string }; + }; + expect(parsed.toolPermissions.default).toBe("ask"); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("src/**")); + }); + + it("round-trips the tool-wide default through import", async () => { + const dirPath = join(testDir, ".rovodev"); + await ensureDir(dirPath); + await writeFileContent( + join(dirPath, "config.yml"), + dump({ toolPermissions: { default: "deny" } }), + ); + + const perms = await RovodevPermissions.fromFile({ outputRoot: testDir, global: true }); + const imported = JSON.parse(perms.toRulesyncPermissions().getFileContent()); + + expect(imported.permission["*"]).toEqual({ "*": "deny" }); + }); + }); + + describe("import falls back to the implicit level for silent tool keys", () => { + it("does not widen a category from one 'always allow' answer", async () => { + const dirPath = join(testDir, ".rovodev"); + await ensureDir(dirPath); + // The shape Rovo Dev leaves behind when the user answers "always allow" + // to a single create_file prompt. + await writeFileContent( + join(dirPath, "config.yml"), + dump({ toolPermissions: { tools: { create_file: "allow" } } }), + ); + + const perms = await RovodevPermissions.fromFile({ outputRoot: testDir, global: true }); + const imported = JSON.parse(perms.toRulesyncPermissions().getFileContent()); + + // Rovo Dev's own default for the silent siblings is `ask`, so the category + // collapses to `ask` rather than handing every mutation tool an `allow`. + expect(imported.permission.edit["*"]).toBe("ask"); + }); + + it("uses the file's own default as the implicit level", async () => { + const dirPath = join(testDir, ".rovodev"); + await ensureDir(dirPath); + await writeFileContent( + join(dirPath, "config.yml"), + dump({ toolPermissions: { default: "deny", tools: { create_file: "allow" } } }), + ); + + const perms = await RovodevPermissions.fromFile({ outputRoot: testDir, global: true }); + const imported = JSON.parse(perms.toRulesyncPermissions().getFileContent()); + + expect(imported.permission.edit["*"]).toBe("deny"); + }); + + it("still imports a fully stated category at its stated level", async () => { + const dirPath = join(testDir, ".rovodev"); + await ensureDir(dirPath); + await writeFileContent( + join(dirPath, "config.yml"), + dump({ + toolPermissions: { + tools: { + find_and_replace_code: "allow", + create_file: "allow", + delete_file: "allow", + move_file: "allow", + createTechnicalPlan: "allow", + createJiraIssue: "allow", + updateJiraIssue: "allow", + createConfluencePage: "allow", + updateConfluencePage: "allow", + }, + }, + }), + ); + + const perms = await RovodevPermissions.fromFile({ outputRoot: testDir, global: true }); + const imported = JSON.parse(perms.toRulesyncPermissions().getFileContent()); + + expect(imported.permission.edit["*"]).toBe("allow"); + }); + + it("invents no rule for a category the file says nothing about", async () => { + const dirPath = join(testDir, ".rovodev"); + await ensureDir(dirPath); + await writeFileContent( + join(dirPath, "config.yml"), + dump({ toolPermissions: { tools: { grep: "deny" } } }), + ); + + const perms = await RovodevPermissions.fromFile({ outputRoot: testDir, global: true }); + const imported = JSON.parse(perms.toRulesyncPermissions().getFileContent()); + + expect(imported.permission.edit).toBeUndefined(); + }); + }); }); diff --git a/src/features/permissions/rovodev-permissions.ts b/src/features/permissions/rovodev-permissions.ts index 181eb576b..20326eba0 100644 --- a/src/features/permissions/rovodev-permissions.ts +++ b/src/features/permissions/rovodev-permissions.ts @@ -38,7 +38,20 @@ type RovodevToolPermissionKey = | "open_files" | "expand_code_chunks" | "expand_folder" - | "grep"; + | "grep" + // The planning and Atlassian tools. These reach Jira and Confluence rather + // than the working tree, but they split the same way the file tools do — + // `get*` inspects, `create*`/`update*` mutates — so they ride the canonical + // `read`/`edit` categories rather than inventing a category for them. + // `createTechnicalPlan` is grouped with the mutating tools because it is the + // planning tool that produces an artifact rather than reading one. + | "createTechnicalPlan" + | "getJiraIssue" + | "createJiraIssue" + | "updateJiraIssue" + | "getConfluencePage" + | "createConfluencePage" + | "updateConfluencePage"; // Maps a rulesync canonical category to the Rovo Dev per-tool keys it controls. // The mapping is intentionally conservative: only categories with a clean Rovo @@ -47,9 +60,36 @@ type RovodevToolPermissionKey = // - `edit`/`write` -> the file mutation tools. // `bash` is handled separately (it owns `bash.default`/`bash.commands`). const CATEGORY_TO_TOOL_KEYS: Record = { - read: ["open_files", "expand_code_chunks", "expand_folder", "grep"], - edit: ["find_and_replace_code", "create_file", "delete_file", "move_file"], - write: ["create_file", "delete_file", "move_file", "find_and_replace_code"], + read: [ + "open_files", + "expand_code_chunks", + "expand_folder", + "grep", + "getJiraIssue", + "getConfluencePage", + ], + edit: [ + "find_and_replace_code", + "create_file", + "delete_file", + "move_file", + "createTechnicalPlan", + "createJiraIssue", + "updateJiraIssue", + "createConfluencePage", + "updateConfluencePage", + ], + write: [ + "create_file", + "delete_file", + "move_file", + "find_and_replace_code", + "createTechnicalPlan", + "createJiraIssue", + "updateJiraIssue", + "createConfluencePage", + "updateConfluencePage", + ], }; // Reverse of CATEGORY_TO_TOOL_KEYS: which canonical category a Rovo Dev tool key @@ -61,10 +101,17 @@ const TOOL_KEY_TO_CATEGORY: Record = expand_code_chunks: "read", expand_folder: "read", grep: "read", + getJiraIssue: "read", + getConfluencePage: "read", find_and_replace_code: "edit", create_file: "edit", delete_file: "edit", move_file: "edit", + createTechnicalPlan: "edit", + createJiraIssue: "edit", + updateJiraIssue: "edit", + createConfluencePage: "edit", + updateConfluencePage: "edit", }; // Every per-tool key rulesync writes, from both directions of the mapping so a @@ -79,7 +126,7 @@ const MANAGED_TOOL_KEYS: readonly RovodevToolPermissionKey[] = [ // Keys directly under `toolPermissions` that rulesync rewrites from // `.rulesync/permissions.*` on every generate. `tools` is handled separately // because only the managed tool keys inside it are owned. -const OWNED_TOOL_PERMISSION_KEYS = ["bash", "allowedExternalPaths"] as const; +const OWNED_TOOL_PERMISSION_KEYS = ["bash", "allowedExternalPaths", "default"] as const; type RovodevBashCommand = { command: string; @@ -92,6 +139,13 @@ type RovodevBash = { }; type RovodevToolPermissions = { + /** + * The level applied to any tool with no more specific setting. Rovo Dev + * defaults it to `ask`. + * + * @see https://support.atlassian.com/rovo/docs/manage-rovo-dev-cli-settings/ + */ + default?: PermissionAction; bash?: RovodevBash; allowedExternalPaths?: string[]; /** @@ -116,9 +170,16 @@ type RovodevToolPermissions = { * Mapping decisions (rulesync canonical -> Rovo Dev): * - `bash`: the catch-all `*` pattern -> `bash.default`; every other pattern -> * a `bash.commands[]` entry `{ command: , permission }`. + * - the all-tools category `*`: its catch-all -> `toolPermissions.default`, + * the level Rovo Dev falls back to for any tool with no more specific + * setting (Rovo Dev's own default is `ask`). * - `read` -> the inspection tools (`open_files`, `expand_code_chunks`, - * `expand_folder`, `grep`); `edit`/`write` -> the mutation tools - * (`find_and_replace_code`, `create_file`, `delete_file`, `move_file`). + * `expand_folder`, `grep`, `getJiraIssue`, `getConfluencePage`); + * `edit`/`write` -> the mutation tools (`find_and_replace_code`, + * `create_file`, `delete_file`, `move_file`, `createTechnicalPlan`, + * `createJiraIssue`, `updateJiraIssue`, `createConfluencePage`, + * `updateConfluencePage`) — so these two categories reach Jira and + * Confluence, not just the working tree. * These Rovo Dev keys hold a single level (no per-pattern rules), so only the * catch-all `*` of each category sets the level. Non-catch-all `allow` rules * in those categories are surfaced as `allowedExternalPaths` so explicit path @@ -429,6 +490,14 @@ function stripPermissiveOwnedValues(toolPermissions: Record): s strippedKeys.push("allowedExternalPaths"); } + // A tool-wide `allow` default is the widest grant in the file, so it goes + // the same way `bash.default: allow` does — dropping it hands the decision + // back to Rovo Dev's own `ask` default. + if (toolPermissions.default === "allow") { + delete toolPermissions.default; + strippedKeys.push("default"); + } + const bash = toolPermissions.bash; if (isRecord(bash)) { const stripped: Record = { ...bash }; @@ -473,22 +542,17 @@ function convertRulesyncToRovodevToolPermissions({ const toolPermissions: RovodevToolPermissions = {}; const allowedExternalPaths: string[] = []; - // `edit` and `write` collapse onto the same Rovo Dev file-mutation tools, so a - // conflicting catch-all between them cannot be represented. Warn that the loss - // is happening, and keep the stricter of the two — the same fail-closed rule - // the import direction uses when those tools disagree, so the resolution never - // grants more than the author asked for. - const editCatchAll = config.permission.edit?.[CATCH_ALL_PATTERN]; - const writeCatchAll = config.permission.write?.[CATCH_ALL_PATTERN]; - if (editCatchAll && writeCatchAll && editCatchAll !== writeCatchAll) { - logger?.warn( - `Rovo Dev maps both "edit" and "write" onto the same file-mutation tools, but they have ` + - `conflicting catch-all permissions ("edit": "${editCatchAll}", "write": "${writeCatchAll}"). ` + - `The stricter of the two ("${strictestAction(editCatchAll, writeCatchAll)}") is used.`, - ); - } + warnOnEditWriteConflict({ config, logger }); for (const [category, rules] of Object.entries(config.permission)) { + if (category === CATCH_ALL_PATTERN) { + const toolWideDefault = convertAllToolsRules({ rules, logger }); + if (toolWideDefault) { + toolPermissions.default = toolWideDefault; + } + continue; + } + if (category === "bash") { const bash = convertBashRules(rules); if (bash) { @@ -536,6 +600,59 @@ function convertRulesyncToRovodevToolPermissions({ return toolPermissions; } +/** + * `edit` and `write` collapse onto the same Rovo Dev file-mutation tools, so a + * conflicting catch-all between them cannot be represented. Warn that the loss + * is happening; the conversion keeps the stricter of the two — the same + * fail-closed rule the import direction uses when those tools disagree, so the + * resolution never grants more than the author asked for. + */ +function warnOnEditWriteConflict({ + config, + logger, +}: { + config: PermissionsConfig; + logger?: Logger | undefined; +}): void { + const editCatchAll = config.permission.edit?.[CATCH_ALL_PATTERN]; + const writeCatchAll = config.permission.write?.[CATCH_ALL_PATTERN]; + if (editCatchAll && writeCatchAll && editCatchAll !== writeCatchAll) { + logger?.warn( + `Rovo Dev maps both "edit" and "write" onto the same file-mutation tools, but they have ` + + `conflicting catch-all permissions ("edit": "${editCatchAll}", "write": "${writeCatchAll}"). ` + + `The stricter of the two ("${strictestAction(editCatchAll, writeCatchAll)}") is used.`, + ); + } +} + +/** + * The canonical all-tools category. Its catch-all sets the tool-wide + * `toolPermissions.default`, the same way `bash`'s catch-all sets + * `bash.default` — both are the level Rovo Dev falls back to. Pattern rules + * under `*` have no counterpart (the default is a single level), so they are + * reported and skipped like any other rule Rovo Dev cannot express. + */ +function convertAllToolsRules({ + rules, + logger, +}: { + rules: Record; + logger?: Logger | undefined; +}): PermissionAction | undefined { + let toolWideDefault: PermissionAction | undefined; + for (const [pattern, action] of Object.entries(rules)) { + if (pattern === CATCH_ALL_PATTERN) { + toolWideDefault = action; + continue; + } + logger?.warn( + `Rovo Dev's tool-wide default is a single level, so it cannot express the ` + + `pattern "${pattern}" in the "*" category. Skipping it.`, + ); + } + return toolWideDefault; +} + function convertBashRules(rules: Record): RovodevBash | undefined { const bash: RovodevBash = {}; const commands: RovodevBashCommand[] = []; @@ -563,6 +680,12 @@ function convertRovodevToolPermissionsToRulesync( ): PermissionsConfig { const permission: PermissionsConfig["permission"] = {}; + // The tool-wide fallback level comes back as the canonical all-tools category, + // mirroring how `bash.default` comes back as `bash`'s catch-all. + if (isPermissionAction(toolPermissions.default)) { + permission[CATCH_ALL_PATTERN] = { [CATCH_ALL_PATTERN]: toolPermissions.default }; + } + const bash = toolPermissions.bash; if (isRecord(bash)) { const bashRules: Record = {}; @@ -588,23 +711,45 @@ function convertRovodevToolPermissionsToRulesync( // Read from `toolPermissions.tools`, falling back to the top level for a file // an earlier rulesync wrote at the wrong depth, so those still import. const nestedTools = isRecord(toolPermissions.tools) ? toolPermissions.tools : {}; - for (const [toolKey, category] of Object.entries(TOOL_KEY_TO_CATEGORY)) { + // The level Rovo Dev applies to a tool key the file says nothing about. + const implicitLevel: PermissionAction = isPermissionAction(toolPermissions.default) + ? toolPermissions.default + : "ask"; + + for (const category of new Set(Object.values(TOOL_KEY_TO_CATEGORY))) { // `tools` is what Rovo Dev actually reads, so a key present there settles // the level even when its value is unusable; the legacy flat copy is only // consulted when `tools` says nothing about the key at all. - const value = Object.hasOwn(nestedTools, toolKey) - ? nestedTools[toolKey] - : toolPermissions[toolKey]; - if (!isPermissionAction(value)) { + const levels = (Object.entries(TOOL_KEY_TO_CATEGORY) as [RovodevToolPermissionKey, string][]) + .filter(([, mapped]) => mapped === category) + .map(([toolKey]) => { + const value = Object.hasOwn(nestedTools, toolKey) + ? nestedTools[toolKey] + : toolPermissions[toolKey]; + return isPermissionAction(value) ? value : undefined; + }); + + // A category the file says nothing about at all is not a rule; giving it the + // implicit level here would invent one for every category in every file. + if (levels.every((level) => level === undefined)) { continue; } + + // A category maps onto several tool keys that can disagree — Rovo Dev + // rewrites a single key when the user answers "always allow" to one prompt. + // They collapse back onto one catch-all here, taking the strictest rather + // than whichever key is iterated last, which would quietly widen the rest. + // + // A key the file is silent about counts as the implicit level rather than + // as absent. Without that, one "always allow" answer about `create_file` + // would import as a blanket `edit: allow`, and the next generate would hand + // that grant to every other tool of the category — Jira and Confluence + // writes included, now that they ride the same category. permission[category] ??= {}; - // A category maps onto four tool keys that can disagree — Rovo Dev rewrites - // a single key when the user answers "always allow" to one prompt. They - // collapse back onto one catch-all here, so take the strictest rather than - // whichever key is iterated last, which would quietly widen the other three. - const current = permission[category][CATCH_ALL_PATTERN]; - permission[category][CATCH_ALL_PATTERN] = strictestAction(current, value); + permission[category][CATCH_ALL_PATTERN] = levels.reduce( + (strictest, level) => strictestAction(strictest, level ?? implicitLevel)!, + permission[category][CATCH_ALL_PATTERN]!, + ); } if (isStringArray(toolPermissions.allowedExternalPaths)) { diff --git a/src/types/tool-target-tuples.ts b/src/types/tool-target-tuples.ts index 7a7c35e41..becc2a0e3 100644 --- a/src/types/tool-target-tuples.ts +++ b/src/types/tool-target-tuples.ts @@ -275,7 +275,13 @@ export const permissionsProcessorToolTargetTuple = [ "zed", ] as const; -export const checksProcessorToolTargetTuple = ["amp", "cursor", "hermesagent", "takt"] as const; +export const checksProcessorToolTargetTuple = [ + "amp", + "cursor", + "hermesagent", + "rovodev", + "takt", +] as const; export const ALL_TOOL_TARGET_TUPLES = [ rulesProcessorToolTargetTuple,