Skip to content

fix(install): prefix language-specific rules to prevent overwriting common rules (rebased #651)#687

Closed
affaan-m wants to merge 3 commits intomainfrom
community/pr-651-rule-collision
Closed

fix(install): prefix language-specific rules to prevent overwriting common rules (rebased #651)#687
affaan-m wants to merge 3 commits intomainfrom
community/pr-651-rule-collision

Conversation

@affaan-m
Copy link
Copy Markdown
Owner

@affaan-m affaan-m commented Mar 20, 2026

Summary

Original PR

Closes #651

Test plan

  • Rebased cleanly onto main
  • All tests pass (only pre-existing observe.sh failure remains)
  • Verify install script correctly prefixes language-specific rules

Summary by cubic

Fix install to prefix language-specific rule files (typescript-, python-, golang-) so they no longer overwrite common rules. Updates the configure-ecc skill docs (EN/JA/ZH) and syncs catalog counts.

  • Bug Fixes
    • Prefix language rule copies into $TARGET/rules/<lang>-<file>; add -r to cp, quote $ECC_ROOT, and guard empty globs.
    • Add troubleshooting note about prefixed file names.
    • Update AGENTS/README counts to 113 skills and 58 commands.

Written for commit 7de2f73. Summary will update on new commits.

tonymfer and others added 3 commits March 20, 2026 00:56
…ommon rules

The configure-ecc skill's Step 3 flat-copies language-specific rule files
(e.g., rules/python/*) into the same $TARGET/rules/ directory as common
rules. Since both directories contain identically-named files
(coding-style.md, security.md, testing.md, hooks.md, patterns.md), the
last language installed silently overwrites common rules.

Fix by prefixing language-specific files during copy (e.g.,
python-coding-style.md, golang-testing.md). This matches the approach
already used by the Antigravity install target adapter.

Also updates the Japanese and Chinese translations of the skill, and
adds a troubleshooting note about the prefixed naming convention.

Closes #432

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ands

Address review feedback:
- Add `-r` flag to `cp` to handle potential subdirectories
- Quote `$ECC_ROOT` in glob to prevent word-splitting on paths with spaces
- Add `[ -e "$f" ]` guard to skip when glob doesn't match anything

Applied consistently across EN, JA, and ZH skill docs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 20, 2026

Warning

Rate limit exceeded

@affaan-m has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 7 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f6f3af9c-55ea-4bc7-8131-71c5ee7363a6

📥 Commits

Reviewing files that changed from the base of the PR and between 07f6156 and 7de2f73.

📒 Files selected for processing (5)
  • AGENTS.md
  • README.md
  • docs/ja-JP/skills/configure-ecc/SKILL.md
  • docs/zh-CN/skills/configure-ecc/SKILL.md
  • skills/configure-ecc/SKILL.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch community/pr-651-rule-collision
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 5 files

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 20, 2026

Greptile Summary

This PR fixes a file-collision bug in the configure-ecc installer where language-specific rule files (e.g. coding-style.md, hooks.md, patterns.md, security.md, testing.md) shared identical names with the common rules and would silently overwrite them during a flat install; it also bumps the catalog counts (109→113 skills, 57→58 commands) to match the actual directory contents.

  • Core fix (skills/configure-ecc/SKILL.md): replaces the three cp -r rules/<lang>/* $TARGET/rules/ one-liners with guarded for loops that prefix each copied file with the language name (e.g. typescript-coding-style.md), correctly preventing the overwrite. The guard [ -e "$f" ] also handles empty or nonexistent source directories gracefully.
  • Translations updated (docs/ja-JP/… and docs/zh-CN/…): the new copy commands are faithfully reflected in both translations, but the companion troubleshooting bullet added to the English file ("Language-specific rules are prefixed: …") was not propagated to either translation.
  • Count updates (AGENTS.md, README.md): all three references to skill/command counts in both files are updated consistently and verified correct against actual directory contents.

Confidence Score: 4/5

  • Safe to merge — the core fix is correct and well-guarded; only a minor translation sync gap exists.
  • The bash fix is logically sound (verified by checking that all five common-rule filenames overlap with each language-specific rules directory), counts are accurate, and the change is documentation-only with no runtime code affected. The only gap is that the new troubleshooting bullet was added to the English SKILL.md but not to the ja-JP or zh-CN translations.
  • docs/ja-JP/skills/configure-ecc/SKILL.md and docs/zh-CN/skills/configure-ecc/SKILL.md — both are missing the new troubleshooting note about prefixed filenames.

Important Files Changed

Filename Overview
skills/configure-ecc/SKILL.md Core fix: replaces flat cp -r for language rules with a prefixed for loop to prevent overwriting common rules sharing the same filenames (confirmed overlap: coding-style.md, hooks.md, patterns.md, security.md, testing.md). New troubleshooting note correctly added.
docs/ja-JP/skills/configure-ecc/SKILL.md Copy commands correctly updated to use prefixed for-loop, but the companion troubleshooting note ("Language-specific rules are prefixed…") added to the English SKILL.md was not propagated to this translation.
docs/zh-CN/skills/configure-ecc/SKILL.md Copy commands correctly updated to use prefixed for-loop, but the companion troubleshooting note ("Language-specific rules are prefixed…") added to the English SKILL.md was not propagated to this translation.
AGENTS.md Catalog counts updated from 109 skills/57 commands to 113 skills/58 commands — verified correct against actual directory contents.
README.md All three catalog count references updated consistently (install success message + comparison table), matching verified directory counts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User runs configure-ecc] --> B[Clone ECC repo to /tmp]
    B --> C[Choose install level\nuser / project / both]
    C --> D[Select & install skills]
    D --> E[Select rule sets]
    E --> F{Which rules?}
    F -->|Common selected| G["cp -r rules/common/* → TARGET/rules/\n(flat copy, no prefix)"]
    F -->|TypeScript selected| H["for f in rules/typescript/*\ncp → TARGET/rules/typescript-basename-f\n(prefixed)"]
    F -->|Python selected| I["for f in rules/python/*\ncp → TARGET/rules/python-basename-f\n(prefixed)"]
    F -->|Go selected| J["for f in rules/golang/*\ncp → TARGET/rules/golang-basename-f\n(prefixed)"]
    G --> K[Post-install verification]
    H --> K
    I --> K
    J --> K
    K --> L{Issues found?}
    L -->|Yes| M[Report file + line + fix]
    L -->|No| N[Optional optimization]
    M --> N
    N --> O[Print summary & cleanup /tmp]

    style G fill:#90EE90
    style H fill:#87CEEB
    style I fill:#87CEEB
    style J fill:#87CEEB
Loading

Comments Outside Diff (1)

  1. docs/ja-JP/skills/configure-ecc/SKILL.md, line 293-295 (link)

    P2 Missing troubleshooting note in translation

    The main skills/configure-ecc/SKILL.md (line 364) gained a new troubleshooting bullet explaining that language-specific rules are now prefixed:

    - Language-specific rules are prefixed: `$TARGET/rules/python-coding-style.md` (correct) vs `$TARGET/rules/coding-style.md` (incorrect — overwrites the common rule)
    

    This note is absent from the ja-JP translation's "ルールが機能しません" section (line 293–295). Without it, Japanese-locale users following only the translated docs will have no hint about the new prefixed filename convention when debugging a broken rules install.

    The zh-CN translation (docs/zh-CN/skills/configure-ecc/SKILL.md, around line 373–376) is missing the same note in its "规则不工作" section.

Last reviewed commit: "fix: update catalog ..."

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7de2f7374a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +224 to +228
# Language-specific rules (prefixed to avoid overwriting common rules)
# Files like coding-style.md become typescript-coding-style.md, etc.
for f in "$ECC_ROOT"/rules/typescript/*; do [ -e "$f" ] && cp -r "$f" "$TARGET/rules/typescript-$(basename "$f")"; done # if selected
for f in "$ECC_ROOT"/rules/python/*; do [ -e "$f" ] && cp -r "$f" "$TARGET/rules/python-$(basename "$f")"; done # if selected
for f in "$ECC_ROOT"/rules/golang/*; do [ -e "$f" ] && cp -r "$f" "$TARGET/rules/golang-$(basename "$f")"; done # if selected
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep Claude rule installs in common/ and language subdirs

The new copy loop still flattens language-specific rules into $TARGET/rules/ and only renames the files. For the Claude Code user/project installs this skill targets, scripts/lib/install-executor.js:16-42 copies rules/common/ and rules/<language>/ as subdirectories, and rules/README.md:45-49 explicitly warns that flattening breaks the ../common/... links embedded in every language rule (for example rules/python/testing.md:8). If an agent follows these updated instructions, it will produce a rule layout the supported installer never creates and leave the cross-references in the installed docs broken.

Useful? React with 👍 / 👎.

@affaan-m
Copy link
Copy Markdown
Owner Author

Duplicate of #690

@affaan-m affaan-m marked this as a duplicate of #690 Mar 20, 2026
@affaan-m affaan-m closed this Mar 20, 2026
@affaan-m affaan-m deleted the community/pr-651-rule-collision branch March 20, 2026 08:42
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