Skip to content

fix: add claudecode to valid targets in parser validation#2

Merged
cm-dyoshikawa merged 1 commit into
dyoshikawa:mainfrom
ise:feature/addtarget
Jun 25, 2025
Merged

fix: add claudecode to valid targets in parser validation#2
cm-dyoshikawa merged 1 commit into
dyoshikawa:mainfrom
ise:feature/addtarget

Conversation

@ise
Copy link
Copy Markdown
Contributor

@ise ise commented Jun 25, 2025

Summary

Fix validation error when importing from CLAUDE.md by adding claudecode to valid targets.

Problem

  • When using the import command to import from CLAUDE.md, the targets field is set to ["claudecode"]
  • However, src/core/parser.ts validation only allows ["copilot", "cursor", "cline", "claude", "roo", "*"]
  • This causes a validation error: Invalid target "claudecode" when running rulesync validate

Solution

  • Added claudecode to the validTargets array in src/core/parser.ts
  • Since claude is already being used as a target in existing configurations, we preserve both claude and claudecode for backward compatibility
  • Added comprehensive tests to ensure both single and mixed target configurations work properly

Changes

  • ✅ Updated validTargets array to include claudecode
  • ✅ Added test cases for claudecode target validation
  • ✅ Added test cases for mixed targets including claudecode
  • ✅ All existing tests pass (159/159)

Testing

pnpm test  # All tests pass

The fix resolves the validation error while maintaining backward compatibility with existing claude target configurations.


Thank you for creating such an amazing tool! 🚀
Please review when you have a chance.

Previously, the claudecode parser would generate frontmatter with
targets: ["claudecode"], but the validation logic only allowed
["copilot", "cursor", "cline", "claude", "roo", "*"], causing
validation errors when importing from Claude Code.

This commit:
- Adds "claudecode" to the validTargets array in parser.ts
- Adds comprehensive tests for claudecode target validation
- Ensures both "claude" and "claudecode" are valid targets

Fixes the issue where `rulesync validate` would fail with:
"Invalid target 'claudecode' in .rulesync/claudecode__claude-main.md"
@cm-dyoshikawa
Copy link
Copy Markdown
Collaborator

Good catch!

I will convert claude keyword to claudecode and delete all claude in this tool's specification in the near future.

Thank you so much.

@cm-dyoshikawa
Copy link
Copy Markdown
Collaborator

I will merged it after reviewing soon.

@cm-dyoshikawa cm-dyoshikawa merged commit 3af4d86 into dyoshikawa:main Jun 25, 2025
1 check passed
dyoshikawa added a commit that referenced this pull request Mar 30, 2026
- Make DISALLOWED_ROVODEV_MODULAR_RULE_BASENAMES case-insensitive to
  prevent bypass on macOS/Windows filesystems (#7)
- Scope AGENTS.local.md gitignore entry to rovodev target instead of
  common, since it is Rovodev-specific (#5)
- Extract buildDeletionRulesFromPaths helper in rules-processor.ts to
  deduplicate the file-discovery-to-deletion pattern (#2)
- Split RovodevRule.fromFile into fromModularFile/fromRootFile private
  helpers for improved readability (#4)
- Add clarifying comment for nonRootPathsForImport unused on forDeletion
  path (#6)

Closes #1392

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dyoshikawa added a commit that referenced this pull request Mar 30, 2026
- Add explanatory comments in ai-file.ts and ai-dir.ts for why
  .replace() is used instead of path.posix.join (#1)
- Improve fetch.test.ts with parameterized Windows-style backslash
  path test inputs via it.each (#2)
- Normalize backslashes in fetch.ts resolvedPath for API compatibility
- Update coding-guidelines.md to distinguish filesystem paths
  (path.join) from semantic/API paths (path.posix.join) (#3)

Closes #1394

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dyoshikawa added a commit that referenced this pull request Mar 31, 2026
…d gitignore sync

- Enforce both-or-neither opts in buildDeletionRulesFromPaths (#1)
- Rename destructured param in fromRootFile for clarity (#2)
- Remove redundant optional chaining after nonRoot guard (#3)
- Remove duplicate **/.rovodev/ gitignore entry (#4)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dyoshikawa added a commit that referenced this pull request Apr 8, 2026
- Add logger to ToolHooksFromRulesyncHooksParams and pass this.logger
  from HooksProcessor so converter warnings actually fire (#1).
- Add passthroughNameDescription flag to ToolHooksConverterConfig and
  enable it only for codexcli/geminicli, preventing the unconditional
  pass-through from leaking unknown 'name'/'description' keys into
  Claude Code / Factory Droid hook outputs (#2).
- Document the Codex CLI command-passthrough behavior for hand-edited
  configs containing other tools' project-dir variables (#3).

Refs #1445

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dyoshikawa added a commit that referenced this pull request Apr 15, 2026
- #1: use static import for resetDeprecationWarningForTests in tests
- #2: document why mutual-exclusivity is runtime-enforced, not a discriminated union
- #3: stop emitting the deprecation warning from the Config constructor;
  the ConfigResolver is now the single emission point
- #4: cache validated ToolTarget[] for object-form targets in the constructor
  so getTargets() no longer rebuilds the ALL_TOOL_TARGETS set per call
- #5: fix misleading schema comment that claimed unknown-target warnings
  (the runtime path actually throws)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dyoshikawa added a commit that referenced this pull request Apr 15, 2026
- #1: use static import for resetDeprecationWarningForTests in tests
- #2: document why mutual-exclusivity is runtime-enforced, not a discriminated union
- #3: stop emitting the deprecation warning from the Config constructor;
  the ConfigResolver is now the single emission point
- #4: cache validated ToolTarget[] for object-form targets in the constructor
  so getTargets() no longer rebuilds the ALL_TOOL_TARGETS set per call
- #5: fix misleading schema comment that claimed unknown-target warnings
  (the runtime path actually throws)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dyoshikawa added a commit that referenced this pull request Apr 21, 2026
…nd match-all bypasses

- Reject imported rules whose toolName maps to __proto__, constructor,
  or prototype to prevent prototype pollution when round-tripping
  untrusted TOML; use Object.hasOwn for lookups to avoid hitting
  inherited accessors. (Sec #1)
- Stop translating glob character classes to regex classes; emit '['
  and ']' as literals so that negated ([^a]) or wide-range ([!-~])
  classes cannot bypass the JSON field-boundary guard. (Sec #2)
- Skip empty patterns ('') with a warning (would match every bash
  invocation or nothing for other tools). Skip bash '*' and '**'
  with allow/deny decisions because they would silently grant or
  revoke every shell command; 'ask' remains supported. (Sec #3)
- Update docs to reflect the new guardrails.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dyoshikawa added a commit that referenced this pull request Apr 21, 2026
Address Round 2 review findings for PR #1526:

- HIGH-R2-#1: guard the stale-file cleanup loop in apm-install.ts
  against path traversal. Attacker-controlled deployed_files entries
  with ".." segments or absolute paths are now rejected by shape and
  via checkPathTraversal, with a warn log per offending entry, so a
  hostile lockfile cannot drive arbitrary removeFile calls.
- MID-R2-#2: make lockfile ordering deterministic for failed deps.
  The per-dep worker now returns the preserved prior entry via its
  result object, and the sequential post-loop pushes successes or
  preserved entries strictly in manifest order, not in
  promise-completion order.
- MID-R2-#3: preserve top-level loose fields (mcp_servers and any
  looseObject extras) across lockfile rewrites by carrying forward
  existingLock through createEmptyApmLock.
- MID-R2-#4: relax the content_hash schema to accept arbitrary
  strings on parse so a lockfile produced by the upstream apm CLI
  does not break readApmLock. The --frozen integrity check now only
  compares hashes whose shape matches RULESYNC_CONTENT_HASH_REGEX
  and skips comparison otherwise (commit SHA pin still enforces
  integrity).

Tests added for each finding, including a two-dep ordering
regression and a frozen-mode interop check with a legacy
content_hash value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
dyoshikawa added a commit that referenced this pull request Apr 29, 2026
- #1 (mid): augmentcode-permissions sort the COMBINED [generated, preserved] entries so a preserved launch-process deny cannot be shadowed by a generated catch-all allow/ask under first-match-wins. Adds regression test.

- #2 (low): sortAugmentEntries applies fail-closed type priority (deny < ask-user < allow) BEFORE the regex-length heuristic for has-regex entries. Heuristic limits documented in code.

- #3 (low): cline-permissions downgrades translation-loss logs from logger.error to a single aggregated logger.warn per call (project convention; CI gates that treat error lines as failures no longer trip).

- #4 (low): documents in docs/reference/file-formats.md (and synced skills/rulesync/file-formats.md) that Cline allow/deny arrays are owned by rulesync entirely (no preservation), in contrast to Qwen Code and AugmentCode.

- #5 (low): qwencode-permissions forwards a logger to parseQwenPermissionEntry from both call sites (preservation filter and convertQwenToRulesyncPermissions) so the malformed-entry warnings are no longer dead code in production.

- #6 (low): qwencode-permissions.test extends the nested-paren round-trip test to cover sequential parens (Bash(grep (foo) | wc (-l))) and multi-nesting (Bash(echo ((deep)))).
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.

2 participants