Skip to content

implement issue12#14

Merged
dyoshikawa merged 8 commits into
dyoshikawa:mainfrom
arrowkato:feature/implement-issue12
Jul 4, 2025
Merged

implement issue12#14
dyoshikawa merged 8 commits into
dyoshikawa:mainfrom
arrowkato:feature/implement-issue12

Conversation

@arrowkato
Copy link
Copy Markdown
Contributor

resolve #12

implemented the specifications #12 (comment)

@dyoshikawa dyoshikawa requested review from Copilot and dyoshikawa July 4, 2025 13:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds support for a new cursorRuleType field in frontmatter to distinguish four Cursor rule behaviors (always, manual, specificFiles, intelligently). The PR updates parsing logic, markdown generation, tests, and documentation to implement the Issue #12 specification.

  • Introduce cursorRuleType in RuleFrontmatter and propagate it through parsing/generation
  • Enhance parseCursorConfiguration and convertCursorMdcFrontmatter to classify MDC/legacy rules into four types
  • Update generateCursorMarkdown and its tests to output the new frontmatter format
  • Revise core validation and add doc/README updates for Cursor rule types

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/types/rules.ts Add optional cursorRuleType to RuleFrontmatter
src/parsers/cursor.ts Implement convertCursorMdcFrontmatter, update parsing
src/parsers/cursor.test.ts Update parsing tests to expect new rule classifications
src/generators/rules/cursor.ts Rewrite generateCursorMarkdown, add determineCursorRuleType
src/generators/rules/cursor.test.ts Extend generator tests for all four rule types
src/core/parser.ts Allow empty description (relax validation)
README.md / README.ja.md Document cursorRuleType field and behavior
CONTRIBUTING.md / CONTRIBUTING.ja.md Note Cursor parser supports four rule types
Comments suppressed due to low confidence (4)

src/parsers/cursor.ts:68

  • [nitpick] Each return in convertCursorMdcFrontmatter repeats root and targets. Extract a base object with { root: false, targets: ["*"] } and spread it into each branch to reduce duplication.
    return {

src/generators/rules/cursor.ts:95

  • The return type of determineCursorRuleType is declared as string. It should be the union "always"|"manual"|"specificFiles"|"intelligently" (e.g. RuleFrontmatter['cursorRuleType']) to enforce type safety.
function determineCursorRuleType(

src/parsers/cursor.ts:33

  • [nitpick] The regex for unquoted glob patterns is complex and brittle. Consider simplifying it or using a YAML loader schema that accepts unquoted strings, and add inline comments explaining each step.
            // But exclude array literals (starting with [ or already quoted strings)

CONTRIBUTING.md:105

  • [nitpick] The comment about supporting four rule types is misaligned under the file tree. Move it to the end of the cursor.ts line so it clearly annotates that file.
│   │   │                  # Supports 4 rule types: always, manual, specificFiles, intelligently

Comment thread src/generators/rules/cursor.ts
@dyoshikawa
Copy link
Copy Markdown
Owner

@arrowkato Thank you!

@dyoshikawa dyoshikawa merged commit ea1ad38 into dyoshikawa:main Jul 4, 2025
1 check passed
@arrowkato
Copy link
Copy Markdown
Contributor Author

Thank you for your review!

dyoshikawa added a commit that referenced this pull request Apr 29, 2026
…ode/cline/qwencode

Addresses 18 reviewer findings raised on PR #1338. Highlights:

- #1 (critical): AugmentCode non-bash categories now fail-closed. A single
  deny rule in `read`/`edit`/`write`/`webfetch`/`websearch` collapses the
  whole tool to a catch-all `deny` entry; non-`*` allow/ask patterns are
  dropped with an aggregated warning instead of being silently downgraded
  to a catch-all `allow` that would shadow a deny.
- #2 (high): `toolPermissions` are sorted to make AugmentCode's
  first-match-wins evaluation safe — entries with `shellInputRegex` come
  before catch-alls, longer regex first, with deny < ask-user < allow as
  the tiebreaker.
- #5 (mid): existing `launch-process` deny entries are preserved across
  regeneration so a user-added shell deny rule cannot be silently
  downgraded; non-deny launch-process entries are still owned by rulesync.
- #3 (mid): Qwencode generation uses `readFileContentOrNull` (no
  `readOrInitializeFileContent`) so dry-run does not create the `.qwen/`
  directory.
- #4 (mid): Kilo schema parsing is deferred and respects `params.validate`,
  so `forDeletion` and dry-run construction never throw on permissive input.
- #7, #16 (mid/low): Cline drops non-bash / ask rules at `logger.error`
  level (rather than warn) and surfaces a defensive warn on allow/deny
  pattern collisions.
- #8, #13 (mid/low): Qwencode pattern parser uses the LAST `)` so nested
  parentheses (e.g. `Bash(echo (a))`) round-trip; malformed entries warn
  and fall back to `*`.
- #9 (low): Augment non-bash warnings are aggregated once per category.
- #11 (low): Kilo's wholesale-replace of the `permission` object is
  documented in `docs/reference/file-formats.md`.
- #10, #6 (low): glob→regex behaviour and round-trip caveats are
  documented.
- #14 (low): `mergedPermissions` is typed as
  `{ allow?: string[]; ask?: string[]; deny?: string[]; [k: string]: unknown }`.
- #15 (low): redundant Qwencode global-mode equivalence assert removed.
- #17 (low): `permissions-processor.test.ts` gains `loadToolFiles` cases
  for AugmentCode, Cline, Kilo, and Qwencode.

Deferred:
- #12 (kilo home-mock migration): the existing kilo global-mode test
  already passes `outputRoot` directly and does not reach
  `getHomeDirectory()`, so introducing the home-mock pattern adds only
  ceremony without coverage.
- #18 (tool-name-mapping helper extraction): postponed to keep this
  fix focused and to avoid touching files outside the permissions
  feature.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for Cursor rule type

3 participants