Conversation
- Add install.sh for Trae IDE integration - Add uninstall.sh with manifest-based safe removal - Add README.md (English) - Add README.zh-CN.md (Chinese) - Support local and global installation - Support TRAE_ENV=cn for CN environment - Non-destructive installation (won't overwrite existing files) - Manifest-based uninstallation (preserves user files) Change-Id: I9870874e272fffd9e1966d9bc40d20142314b969
|
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds ECC installation and uninstallation tooling and documentation for Trae: English and Chinese README files plus install/uninstall Bash scripts that copy ECC workflow artifacts into a target Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Installer as "install.sh"
participant Repo as "Repo (source files)"
participant Trae as "Target Trae Dir"
participant Manifest as ".ecc-manifest"
rect rgba(200,230,255,0.5)
User->>Installer: run install.sh
Installer->>Repo: locate installer path, read source dirs (commands/ agents/ skills/ rules, READMEs)
Installer->>Trae: create target dirs (.trae or .trae-cn) and subdirs
Installer->>Trae: copy files (preserve structure)
Installer->>Manifest: append installed relative paths (create if missing)
Installer->>User: print summary + uninstall instructions
end
rect rgba(230,255,200,0.5)
User->>Installer: run uninstall.sh
Installer->>Trae: locate target Trae dir, read .ecc-manifest
Installer->>User: prompt for confirmation (if needed)
Installer->>Trae: remove files listed in manifest (skip invalid/outside)
Installer->>Trae: remove empty directories, attempt remove Trae dir if empty
Installer->>User: print removal summary
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Analysis Failed
Troubleshooting
Retry: |
|
拒收此类邮件!
|
There was a problem hiding this comment.
7 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".trae/README.md">
<violation number="1" location=".trae/README.md:80">
P2: Script examples omit required working-directory context, making `./install.sh` commands non-executable when copied from project root.</violation>
</file>
<file name=".trae/README.zh-CN.md">
<violation number="1" location=".trae/README.zh-CN.md:3">
P2: User-facing README adds a direct link to an external repository, which violates the documented policy to avoid unvetted external repo links.</violation>
<violation number="2" location=".trae/README.zh-CN.md:88">
P2: Install examples are path-inconsistent: `./install.sh` is shown without directory context, conflicting with project-root usage of `.trae/install.sh` and likely failing for users.</violation>
</file>
<file name=".trae/install.sh">
<violation number="1" location=".trae/install.sh:73">
P2: Installer wipes existing manifest before a reinstall, but only records newly copied files, causing previously installed files to become untracked for uninstall.</violation>
<violation number="2" location=".trae/install.sh:89">
P2: Installer suppresses copy/chmod errors but still marks items as installed in manifest and counters, causing false success reporting and inaccurate uninstall tracking.</violation>
<violation number="3" location=".trae/install.sh:119">
P2: Skill manifest generation is non-recursive while skill installation is recursive, leading to incomplete uninstall of nested installed content.</violation>
</file>
<file name=".trae/uninstall.sh">
<violation number="1" location=".trae/uninstall.sh:104">
P1: Manifest paths are used for file deletion without boundary checks, allowing `..` traversal to remove files outside the `.trae` directory.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Hi @affaan-m 👋 Thanks for considering this PR! This initial commit adds .trae/ support to bring ECC workflows to Trae IDE. Current scope:
Planned for future PRs:
Happy to iterate based on your feedback! 🚀 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.trae/install.sh:
- Around line 130-144: The installer currently loops only over
"$REPO_ROOT/rules/common" and copies files into a flat "$trae_full_path/rules/"
which breaks relative ../common links and omits language subdirectories; change
the logic to recursively copy the entire rules tree while preserving directory
structure (e.g. use a recursive copy like cp -a or rsync -a from
"$REPO_ROOT/rules" to "$trae_full_path/rules"), skip overwriting existing files,
and update the MANIFEST with the relative path for each installed file while
incrementing the rules counter for each added file (refer to the loop and
variables: REPO_ROOT, trae_full_path, MANIFEST, and rules).
- Around line 71-74: The install script unconditionally removes the manifest
(MANIFEST variable) which causes previously installed files skipped during
reinstall to never be recorded and thus not removed by uninstall; modify the
logic around MANIFEST (the rm -f "$MANIFEST" call) so the manifest is not wiped
on reinstall — either remove that rm entirely or only remove the manifest when
doing a fresh install (detect by checking for an existing .trae directory or a
--fresh/--force flag), and when skipping existing files ensure they remain
listed in the manifest (append or merge existing manifest entries instead of
discarding them) so uninstall.sh can reliably remove all installed files.
- Around line 118-125: The manifest loop only records direct children of
"$target_skill_dir" so nested files are missed; change the recording to walk
"$target_skill_dir" recursively (e.g., use find to list all regular files under
"$target_skill_dir") and append each entry as
"skills/$skill_name/<path-relative-to-$target_skill_dir>" to "$MANIFEST"
(replace the current for skill_file...basename logic), preserving the same
behavior for errors and keeping the final "skills/$skill_name" entry and the
skills counter increment.
In @.trae/uninstall.sh:
- Around line 101-125: The manifest entries are not validated and can cause path
traversal; before using file_path to build full_path (in the while loop that
reads MANIFEST and when referencing tra e_full_path and file_path), reject
entries that are absolute (start with /), contain parent-directory components
(..), or expand home (~), and additionally normalize the candidate path with
realpath -m (or readlink -f fallback) to produce a
resolved_full="$realpath($trae_full_path/$file_path)"; then ensure resolved_full
has tra e_full_path as its prefix (e.g., resolved_full = tra e_full_path or
starts with tra e_full_path/); if the check fails, treat the entry as invalid:
log a "Skipped: $file_path (invalid manifest entry)" message and increment
skipped, and only proceed to rm/rmdir when the resolved_full is inside tra
e_full_path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e0bcdf0-e63f-4db4-8f9c-1eb1468d2451
📒 Files selected for processing (4)
.trae/README.md.trae/README.zh-CN.md.trae/install.sh.trae/uninstall.sh
|
Analysis Failed
Troubleshooting
Retry: |
|
Analysis Failed
Troubleshooting
Retry: |
Greptile SummaryThis PR adds a Key findings:
Confidence Score: 4/5Safe to merge after fixing the silent argument-ignore bug in install.sh/uninstall.sh; remaining findings are style or documentation quality. One P1 defect: arbitrary path arguments are silently discarded, causing installs to land in $PWD with no warning — a clear behavioral regression vs the existing .kiro/install.sh. The remaining issues (python3 dependency, README gap, local-var pattern) are P2 quality concerns that don't block correctness for the documented use cases. .trae/install.sh and .trae/uninstall.sh — specifically the argument-handling blocks and the python3 dependency in uninstall.sh. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([User runs install.sh]) --> B{Arg provided?}
B -- "~ or $HOME" --> C[target_dir = $HOME]
B -- "any other arg" --> D["⚠️ silently ignored\ntarget_dir = $PWD"]
B -- "no arg" --> E[target_dir = $PWD]
C & D & E --> F{basename of target_dir?}
F -- ".trae or .trae-cn" --> G[trae_full_path = target_dir]
F -- "other" --> H["trae_full_path = target_dir/.trae or .trae-cn"]
G & H --> I[Create subdirs: commands/ agents/ skills/ rules/]
I --> J[Copy files from REPO_ROOT non-destructively]
J --> K[Write / update .ecc-manifest]
K --> L([Done])
M([User runs uninstall.sh]) --> N{Manifest found?}
N -- "No" --> O[Prompt: remove entire dir?]
O -- "Yes" --> P[rm -rf trae_full_path]
O -- "No" --> Q([Cancelled])
N -- "Yes" --> R[Prompt: confirm uninstall?]
R -- "Yes" --> S[Read manifest entries, validate & resolve each path]
S --> T[rm files within trae_root]
T --> U[rmdir empty dirs]
U --> L
R -- "No" --> Q
Reviews (1): Last reviewed commit: "Merge branch 'main' into feature/trae-in..." | Re-trigger Greptile |
| fi | ||
|
|
||
| # Check if we're already inside a .trae or .trae-cn directory | ||
| local current_dir_name="$(basename "$target_dir")" | ||
| local trae_full_path | ||
|
|
||
| if [ "$current_dir_name" = ".trae" ] || [ "$current_dir_name" = ".trae-cn" ]; then | ||
| # Already inside the trae directory, use it directly | ||
| trae_full_path="$target_dir" |
There was a problem hiding this comment.
Arbitrary path arguments silently ignored
When the caller passes a path other than ~ or $HOME (e.g. ./install.sh /some/custom/path), the argument is silently discarded and the install proceeds into $PWD. There is no warning to the user that their argument was ignored.
The existing .kiro/install.sh already handles this correctly:
TARGET="${1:-.}"
if [ "$TARGET" = "~" ] || [[ "$TARGET" == "~/"* ]]; then
TARGET="${TARGET/#\~/$HOME}"
fi
TARGET="$(cd "$TARGET" 2>/dev/null && pwd || echo "$TARGET")"Suggested fix for .trae/install.sh:
if [ "$#" -ge 1 ]; then
local arg="$1"
if [ "$arg" = "~" ] || [[ "$arg" == "~/"* ]]; then
arg="${arg/#\~/$HOME}"
fi
target_dir="$(cd "$arg" 2>/dev/null && pwd || { echo "Error: invalid path: $arg"; exit 1; })"
fiThe same issue exists in .trae/uninstall.sh at the equivalent argument-handling block.
| python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1" | ||
| } | ||
|
|
||
| is_valid_manifest_entry() { |
There was a problem hiding this comment.
resolve_path calls python3, which may not be present in minimal environments (e.g. slim Docker images, CI containers, or embedded systems). The script will fail at runtime with a command not found error without any helpful diagnostic.
Consider using realpath with a python3 fallback:
resolve_path() {
if command -v realpath > /dev/null 2>&1; then
realpath "$1"
elif command -v python3 > /dev/null 2>&1; then
python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1"
else
local dir file
dir="$(dirname "$1")"
file="$(basename "$1")"
echo "$(cd "$dir" && pwd)/$file"
fi
}| ```bash | ||
| # Install to current project | ||
| cd /path/to/your/project | ||
| TRAE_ENV=cn .trae/install.sh | ||
| ``` | ||
|
|
||
| This creates `.trae-cn/` in your project directory. | ||
|
|
||
| ### Option 2: Global Installation (All Projects) | ||
|
|
||
| ```bash | ||
| # Install globally to ~/.trae-cn/ |
There was a problem hiding this comment.
Quick Start "Option 1" conflates local install with CN environment
"Option 1: Local Installation" immediately jumps to TRAE_ENV=cn .trae/install.sh, which installs into .trae-cn/. The default (non-CN) local install has no Quick Start example in the English README.
The Chinese README (README.zh-CN.md) correctly documents all four cases as 方式一–方式四. The English README should mirror that structure:
### Option 1: Local Installation (Default)
```bash
cd /path/to/your/project
.trae/install.shThis creates .trae/ in your project directory.
Option 2: Local Installation (CN Environment)
cd /path/to/your/project
TRAE_ENV=cn .trae/install.shThis creates .trae-cn/ in your project directory.
| # Install function | ||
| do_install() { | ||
| local target_dir="$PWD" | ||
| local trae_dir="$(get_trae_dir)" | ||
|
|
There was a problem hiding this comment.
local var=$(command) masks command exit codes in bash
Under set -euo pipefail, assigning via local var=$(...) does not propagate a non-zero exit code — local always returns 0. The idiomatic fix is to split declaration and assignment:
local trae_dir
trae_dir="$(get_trae_dir)"
local current_dir_name
current_dir_name="$(basename "$target_dir")"This pattern appears throughout both install.sh and uninstall.sh and should be applied consistently.
Summary
Adds native Trae IDE support to ECC via .trae/ directory with install/uninstall scripts and comprehensive documentation.
What's included
.trae/install.sh.trae/uninstall.sh.trae/README.md.trae/README.zh-CN.mdKey Features
Usage
About Trae
Trae is an AI-powered IDE developed by ByteDance (字节跳动), China's leading technology company known for TikTok, Douyin, and other popular apps. Trae integrates AI capabilities directly into the development workflow, providing intelligent code completion, agent assistance, and workflow automation similar to Cursor or GitHub Copilot.
The .trae/ directory enables ECC (Everything Claude Code) workflows to run within Trae IDE, bringing the same battle-tested agents, commands, skills, and rules to Trae users.
Summary by cubic
Adds native Trae IDE support via a
.trae/(or.trae-cn/) folder with install/uninstall scripts so ECC commands, agents, skills, and rules run inside Trae. The flow is non-destructive and validated for safe installs and removals.New Features
install.shanduninstall.shwith non-destructive copy and.ecc-manifest-based safe removal.~/.traeor~/.trae-cn.TRAE_ENV=cnto switch to the CN environment.commands/,agents/,skills/, andrules/inside Trae.Bug Fixes
Written for commit 0d30da1. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation