feat(install): add CodeBuddy(Tencent) adaptation with installation scripts#1038
Merged
affaan-m merged 3 commits intoMar 31, 2026
Merged
Conversation
Contributor
|
Contributor
Analysis Failed
Troubleshooting
Retry: |
Contributor
📝 WalkthroughWalkthroughAdds CodeBuddy as a new install target: docs, cross-platform install/uninstall scripts, a project install-target adapter (with flattened rule handling), schema/manifest updates, registry/tests, and install-state manifest tracking. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Installer as Install Script
participant Repo
participant Adapter as Install-Target Adapter
participant Manifest as .ecc-manifest
participant FS as Target .codebuddy
CLI->>Installer: run install (target path, profile)
Installer->>Repo: enumerate `commands/`,`agents/`,`skills/`,`rules/`, READMEs
Installer->>Adapter: request planOperations(modules, repoRoot, projectRoot)
Adapter->>Adapter: create scaffold ops (flatten `rules` -> namespaced files)
Adapter-->>Installer: planned operations list
Installer->>Manifest: read/update `.ecc-manifest`
Installer->>FS: copy files per plan, mark manifest entries
FS-->>Installer: success counts
Installer->>CLI: print summary and next steps
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
Greptile SummaryThis PR integrates CodeBuddy (Tencent) as a first-class install target by adding a Key findings:
Confidence Score: 4/5Safe to merge after fixing the rule-flattening bug in the legacy install scripts, as the primary adapter-based path is correct. The adapter, registry, schema, manifest, and test changes are all clean and well-structured. However, both legacy scripts (.codebuddy/install.js and .codebuddy/install.sh) have a confirmed P1 defect: they copy rules with the original nested directory hierarchy instead of the flat namespaced layout that CodeBuddy requires and that the README documents. Since these scripts are actively surfaced in the README as a Legacy option, users will follow that path and get silently broken rule installations. This warrants a 4 rather than 5. .codebuddy/install.js and .codebuddy/install.sh — both need their rules-copy loops updated to flatten file paths using the dir-file.md naming convention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User: install ECC for CodeBuddy] --> B{Which path?}
B --> C[Adapter-based - Recommended]
B --> D[Legacy scripts]
C --> E["node scripts/install-apply.js\n--target codebuddy --profile developer"]
E --> F[codebuddy-project adapter\nscripts/lib/install-targets/codebuddy-project.js]
F --> G[resolveRoot → .codebuddy/]
F --> H["planOperations()"]
H --> I{sourceRelativePath == 'rules'?}
I -- Yes --> J["createFlatRuleOperations()\nrules/common/style.md → rules/common-style.md ✅"]
I -- No --> K[createScaffoldOperation\ncommands/ agents/ skills/]
G & J & K --> L[ecc-install-state.json tracks files]
D --> M[".codebuddy/install.sh\nor install.js"]
M --> N[Copy commands/ agents/ skills/]
M --> O["Copy rules preserving hierarchy\nrules/common/style.md → rules/common/style.md ❌"]
O --> P[".ecc-manifest tracks files\nwith wrong paths"]
L --> Q["doctor/repair/uninstall\nvia scripts/doctor.js etc."]
P --> R["Uninstall via\n.codebuddy/uninstall.sh/.js"]
style J fill:#90EE90
style O fill:#FFB6C1
Reviews (3): Last reviewed commit: "fix(codebuddy): resolve installer path i..." | Re-trigger Greptile |
Comment on lines
+19
to
+21
| resolve_path() { | ||
| python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1" | ||
| } |
Contributor
There was a problem hiding this comment.
resolve_path() may break on minimal environments
resolve_path() shells out to python3 to canonicalize paths:
resolve_path() {
python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1"
}Python 3 is not universally available — it is absent from many minimal Docker images (Alpine base, debian:slim), and is not guaranteed on Windows Git Bash environments. If python3 is missing, every manifest entry during uninstall will fail with a command-not-found error, and the script will abort (due to set -euo pipefail).
A more portable alternative:
Suggested change
| resolve_path() { | |
| python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1" | |
| } | |
| resolve_path() { | |
| realpath "$1" 2>/dev/null || readlink -f "$1" 2>/dev/null || python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1" | |
| } |
realpath is part of GNU coreutils (Linux); readlink -f works on GNU/Linux; python3 becomes last-resort.
Contributor
There was a problem hiding this comment.
5 issues found across 12 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=".codebuddy/uninstall.sh">
<violation number="1" location=".codebuddy/uninstall.sh:20">
P2: Uninstall script hard-depends on `python3` without checking availability, causing immediate failure under `set -e` in environments where only `python` (or no Python) is present.</violation>
<violation number="2" location=".codebuddy/uninstall.sh:41">
P1: Unsupported first-argument values are silently ignored, causing uninstall to run on `$PWD` instead of the user-provided target.</violation>
</file>
<file name="manifests/install-modules.json">
<violation number="1" location="manifests/install-modules.json:15">
P1: Added `codebuddy` targets are incompatible with required dependency `platform-configs`, causing codebuddy module resolution to be skipped/fail during install planning.</violation>
</file>
<file name=".codebuddy/uninstall.js">
<violation number="1" location=".codebuddy/uninstall.js:217">
P1: Directory containment check is bypassable via prefix-matching sibling paths, enabling deletion outside `.codebuddy`.</violation>
</file>
<file name=".codebuddy/install.js">
<violation number="1" location=".codebuddy/install.js:103">
P1: Installer reports copied files as successfully installed even when manifest update fails, leaving files unmanaged for uninstall/repair flows.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
|
|
||
| # Check if ~ was specified (or expanded to $HOME) | ||
| if [ "$#" -ge 1 ]; then | ||
| if [ "$1" = "~" ] || [ "$1" = "$HOME" ]; then |
Contributor
There was a problem hiding this comment.
P1: Unsupported first-argument values are silently ignored, causing uninstall to run on $PWD instead of the user-provided target.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/uninstall.sh, line 41:
<comment>Unsupported first-argument values are silently ignored, causing uninstall to run on `$PWD` instead of the user-provided target.</comment>
<file context>
@@ -0,0 +1,183 @@
+
+ # Check if ~ was specified (or expanded to $HOME)
+ if [ "$#" -ge 1 ]; then
+ if [ "$1" = "~" ] || [ "$1" = "$HOME" ]; then
+ target_dir="$HOME"
+ fi
</file context>
| } | ||
|
|
||
| ensureManifestEntry(manifestPath, manifestEntry); | ||
| return true; |
Contributor
There was a problem hiding this comment.
P1: Installer reports copied files as successfully installed even when manifest update fails, leaving files unmanaged for uninstall/repair flows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/install.js, line 103:
<comment>Installer reports copied files as successfully installed even when manifest update fails, leaving files unmanaged for uninstall/repair flows.</comment>
<file context>
@@ -0,0 +1,326 @@
+ }
+
+ ensureManifestEntry(manifestPath, manifestEntry);
+ return true;
+ } catch (err) {
+ console.error(`Error copying ${sourcePath}: ${err.message}`);
</file context>
| CODEBUDDY_DIR=".codebuddy" | ||
|
|
||
| resolve_path() { | ||
| python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1" |
Contributor
There was a problem hiding this comment.
P2: Uninstall script hard-depends on python3 without checking availability, causing immediate failure under set -e in environments where only python (or no Python) is present.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/uninstall.sh, line 20:
<comment>Uninstall script hard-depends on `python3` without checking availability, causing immediate failure under `set -e` in environments where only `python` (or no Python) is present.</comment>
<file context>
@@ -0,0 +1,183 @@
+CODEBUDDY_DIR=".codebuddy"
+
+resolve_path() {
+ python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$1"
+}
+
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
manifests/install-modules.json (1)
147-160:⚠️ Potential issue | 🔴 CriticalUpdate
SUPPORTED_INSTALL_TARGETSto includecodebuddyinscripts/lib/install-manifests.js.Modules in
manifests/install-modules.jsonnow declarecodebuddyas a supported target, butSUPPORTED_INSTALL_TARGETSinscripts/lib/install-manifests.js(line 7) does not include it. The resolver will reject any install plan forcodebuddytarget with "Unknown install target" error. Add'codebuddy'to the array to match the module manifest updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@manifests/install-modules.json` around lines 147 - 160, Update the SUPPORTED_INSTALL_TARGETS array in scripts/lib/install-manifests.js to include 'codebuddy' so it matches manifests/install-modules.json; locate the SUPPORTED_INSTALL_TARGETS constant (around where install targets are defined) and add the string 'codebuddy' to that array literal to prevent the resolver from rejecting install plans for the codebuddy target.
🧹 Nitpick comments (1)
tests/lib/install-targets.test.js (1)
220-287: Consider adding one regression test for target-list parity across config/schema/runtime.These tests validate adapter behavior well, but a parity test (e.g., schema
targetenums vs runtime supported targets) would prevent drift like the currentcodebuddymismatch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/lib/install-targets.test.js` around lines 220 - 287, Add a regression test in tests/lib/install-targets.test.js that loads the config/schema "target" enum values and asserts runtime parity by calling getInstallTargetAdapter(...) for each enum value and verifying the adapter exists and adapter.supports(enumValue) returns true; similarly iterate known runtime targets (e.g., call getInstallTargetAdapter for each runtime target list if available) and assert each is present in the schema enum — use the getInstallTargetAdapter function and the schema's "target" enum symbol to locate values to compare.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codebuddy/install.js:
- Line 195: The variable "other" is declared and incremented for README/script
copies but never used, causing no-unused-vars and hiding whether those assets
were installed; either remove the "other" declaration and all increments
referencing it, or wire it into the installation summary/log so its value is
reported (e.g., include "other" in the final summary/counter output where other
counters are printed). Locate the "other" variable and its increment sites (the
README/script copy logic) and apply one of these two fixes consistently across
all occurrences.
- Around line 148-150: The current installer copies the installers and the
.codebuddy tree into the target so when the copied install.js runs its computed
scriptDir/repoRoot (using scriptDir, repoRoot, codebuddyDirName) points at the
target copy instead of the original packaged sources; fix by not copying the
installers or the .codebuddy folder into the target (exclude install.js,
install.sh and codebuddyDirName in the copy logic) so the relocated install.js
cannot run against a target-local tree, or alternatively change the path
resolution to always resolve packaged sources (keep scriptDir/repoRoot pointing
to the package location rather than the target) if target-local reinstall must
be supported.
In @.codebuddy/install.sh:
- Around line 17-18: The installer computes REPO_ROOT from SCRIPT_DIR which
breaks when install.sh is copied into a target (so subsequent runs read the
target parent); update install.sh to resolve the real ECC package root instead
of the script's current directory by detecting the repository root (e.g., using
git to find the top-level or walking up directories for a known marker file) and
assign that to REPO_ROOT; change references around SCRIPT_DIR and REPO_ROOT in
install.sh so installs always read commands/, agents/, skills/, and rules/ from
the original ECC package root rather than the relocated copy.
In @.codebuddy/README.zh-CN.md:
- Line 75: Update the directory tree comment that currently reads "README.md
# 此文件" to accurately reference this file's actual name; replace "README.md" with
"README.zh-CN.md" or change the comment to something like "README.md (英文说明文件)"
so the label matches the Chinese-localized filename used in this document; edit
the line in .codebuddy/README.zh-CN.md where the tree lists README.md and ensure
the filename and its inline comment are consistent.
In @.codebuddy/uninstall.js:
- Line 17: The variables isWindows and scriptDir are declared but never read,
causing no-unused-vars lint errors; either remove their declarations entirely
or, if they must remain for future use, rename them to _isWindows and _scriptDir
(or prefix with an underscore) to signal intentional unused locals; apply the
same change to the other unused occurrence noted (scriptDir at the later
location) and run the linter to confirm the warnings are resolved.
- Around line 213-245: The current security check and deletion use resolvedFull
which allows sibling paths and can remove symlink targets; replace the
startsWith check with a boundary-aware validation using path.relative (e.g.,
const rel = path.relative(codebuddyRootResolved, path.resolve(fullPath)) and
ensure rel && !rel.startsWith('..') && !path.isAbsolute(rel)), and switch to
operating on the manifest entry path itself: call fs.lstatSync(fullPath) to
detect symlinks and, when removing, always unlink or rmdir the original fullPath
(use fs.unlinkSync(fullPath) for files/symlinks, fs.rmdirSync(fullPath) for
empty dirs) while using resolvePath/resolvedFull only for safe in-repo
resolution checks where needed; update branches that currently use
statSync(resolvedFull), unlinkSync(resolvedFull), readdirSync(resolvedFull), and
rmdirSync(resolvedFull) to use lstatSync/fullPath-based deletes and
statSync/resolvedFull reads only as required.
In @.codebuddy/uninstall.sh:
- Around line 117-147: The uninstall logic currently validates the canonical
path (resolved_full) but then deletes the canonical target; change the removal
operations to act on the recorded manifest path (full_path) instead of
resolved_full: keep the safety case using resolve_path and
codebuddy_root_resolved for validation, but in the -f and -d branches use -f
"$full_path"/-d "$full_path", ls -A "$full_path", rm -f "$full_path" and rmdir
"$full_path" (or tests that rely on full_path) so we remove the manifest
entry/symlink itself rather than the resolved target.
In `@schemas/ecc-install-config.schema.json`:
- Around line 25-27: The schema now allows target "codebuddy" but the runtime
list in the constant SUPPORTED_INSTALL_TARGETS (in
scripts/lib/install-manifests.js) doesn't include "codebuddy", causing runtime
validation to reject valid configs; update SUPPORTED_INSTALL_TARGETS to add the
string "codebuddy" (and any related places that derive behavior from that list,
e.g., validation or switch/case logic that checks SUPPORTED_INSTALL_TARGETS) so
the runtime target list matches the schema.
---
Outside diff comments:
In `@manifests/install-modules.json`:
- Around line 147-160: Update the SUPPORTED_INSTALL_TARGETS array in
scripts/lib/install-manifests.js to include 'codebuddy' so it matches
manifests/install-modules.json; locate the SUPPORTED_INSTALL_TARGETS constant
(around where install targets are defined) and add the string 'codebuddy' to
that array literal to prevent the resolver from rejecting install plans for the
codebuddy target.
---
Nitpick comments:
In `@tests/lib/install-targets.test.js`:
- Around line 220-287: Add a regression test in
tests/lib/install-targets.test.js that loads the config/schema "target" enum
values and asserts runtime parity by calling getInstallTargetAdapter(...) for
each enum value and verifying the adapter exists and adapter.supports(enumValue)
returns true; similarly iterate known runtime targets (e.g., call
getInstallTargetAdapter for each runtime target list if available) and assert
each is present in the schema enum — use the getInstallTargetAdapter function
and the schema's "target" enum symbol to locate values to compare.
🪄 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: 394e2143-5800-4447-8c1a-99148625eca8
📒 Files selected for processing (12)
.codebuddy/README.md.codebuddy/README.zh-CN.md.codebuddy/install.js.codebuddy/install.sh.codebuddy/uninstall.js.codebuddy/uninstall.shmanifests/install-modules.jsonschemas/ecc-install-config.schema.jsonschemas/install-modules.schema.jsonscripts/lib/install-targets/codebuddy-project.jsscripts/lib/install-targets/registry.jstests/lib/install-targets.test.js
Contributor
|
Contributor
Analysis Failed
Troubleshooting
Retry: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/lib/install-manifests.js`:
- Line 7: SUPPORTED_INSTALL_TARGETS now includes 'codebuddy' but the legacy
base-module resolution still falls back to 'claude'; add an explicit mapping
entry for 'codebuddy' in the legacy base-module mapping (the object/function
that maps new targets to legacy base modules, e.g., legacyBaseModuleMap or
resolveLegacyBaseModule) so 'codebuddy' resolves to its correct legacy module
(or to 'codebuddy' itself) instead of defaulting to 'claude'; update the mapping
in the same file (also where the mapping is used around lines ~307-308) to
include the 'codebuddy' key and corresponding value.
🪄 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: 58942fce-55dc-478a-a4da-9f7c01f3ec90
📒 Files selected for processing (2)
manifests/install-modules.jsonscripts/lib/install-manifests.js
✅ Files skipped from review due to trivial changes (1)
- manifests/install-modules.json
|
|
||
| const DEFAULT_REPO_ROOT = path.join(__dirname, '../..'); | ||
| const SUPPORTED_INSTALL_TARGETS = ['claude', 'cursor', 'antigravity', 'codex', 'opencode']; | ||
| const SUPPORTED_INSTALL_TARGETS = ['claude', 'cursor', 'antigravity', 'codex', 'opencode', 'codebuddy']; |
Contributor
There was a problem hiding this comment.
Add explicit legacy base-module mapping for codebuddy to avoid implicit claude fallback.
Now that codebuddy is accepted, legacy selection for target=codebuddy silently falls back to claude base modules. That can pull target-incompatible modules into the request set and produce confusing skips in the resulting plan.
💡 Proposed fix
const LEGACY_COMPAT_BASE_MODULE_IDS_BY_TARGET = Object.freeze({
claude: [
'rules-core',
'agents-core',
'commands-core',
'hooks-runtime',
'platform-configs',
'workflow-quality',
],
cursor: [
'rules-core',
'agents-core',
'commands-core',
'hooks-runtime',
'platform-configs',
'workflow-quality',
],
+ codebuddy: [
+ 'rules-core',
+ 'agents-core',
+ 'commands-core',
+ 'hooks-runtime',
+ 'workflow-quality',
+ ],
antigravity: [
'rules-core',
'agents-core',
'commands-core',
],
});Also applies to: 307-308
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/lib/install-manifests.js` at line 7, SUPPORTED_INSTALL_TARGETS now
includes 'codebuddy' but the legacy base-module resolution still falls back to
'claude'; add an explicit mapping entry for 'codebuddy' in the legacy
base-module mapping (the object/function that maps new targets to legacy base
modules, e.g., legacyBaseModuleMap or resolveLegacyBaseModule) so 'codebuddy'
resolves to its correct legacy module (or to 'codebuddy' itself) instead of
defaulting to 'claude'; update the mapping in the same file (also where the
mapping is used around lines ~307-308) to include the 'codebuddy' key and
corresponding value.
Contributor
|
Contributor
Analysis Failed
Troubleshooting
Retry: |
Contributor
There was a problem hiding this comment.
6 issues found across 6 files (changes from recent commits).
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=".codebuddy/install.sh">
<violation number="1" location=".codebuddy/install.sh:23">
P2: find_repo_root claims to walk up the directory tree but only checks the immediate parent of SCRIPT_DIR, so the “copied script” scenario described in comments will fail and abort installation.</violation>
<violation number="2" location=".codebuddy/install.sh:33">
P2: `set -e` causes the script to exit on a non-zero status from `find_repo_root` inside the command substitution, so the custom "Cannot locate" error block may never run. This makes root-not-found failures less diagnosable.</violation>
<violation number="3" location=".codebuddy/install.sh:196">
P2: Installer now skips copying uninstall.sh but still instructs users to run `./uninstall.sh` in the target directory, so the documented uninstall path will not exist after install.</violation>
</file>
<file name=".codebuddy/uninstall.sh">
<violation number="1" location=".codebuddy/uninstall.sh:122">
P1: Boundary check uses `abspath` (symlink-unsafe) while deletion follows symlinks, allowing files outside `.codebuddy` to be removed via symlinked entries.</violation>
</file>
<file name=".codebuddy/install.js">
<violation number="1" location=".codebuddy/install.js:270">
P2: Installer no longer copies uninstall.js into the target directory but still tells users to run `node uninstall.js` there, so the uninstall instructions will fail on fresh installs.</violation>
</file>
<file name=".codebuddy/uninstall.js">
<violation number="1" location=".codebuddy/uninstall.js:213">
P1: Containment check uses path.resolve instead of resolving symlinks, allowing manifest entries that traverse a symlinked subdirectory to pass the check and delete files outside `.codebuddy`. This regresses the previous realpath-based safety and is security-relevant.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| # Security check: ensure the path resolves inside the target directory. | ||
| # Use Python to compute a reliable relative path so symlinks cannot | ||
| # escape the boundary. | ||
| relative="$(python3 -c 'import os,sys; print(os.path.relpath(os.path.abspath(sys.argv[1]), sys.argv[2]))' "$full_path" "$codebuddy_root_resolved")" |
Contributor
There was a problem hiding this comment.
P1: Boundary check uses abspath (symlink-unsafe) while deletion follows symlinks, allowing files outside .codebuddy to be removed via symlinked entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/uninstall.sh, line 122:
<comment>Boundary check uses `abspath` (symlink-unsafe) while deletion follows symlinks, allowing files outside `.codebuddy` to be removed via symlinked entries.</comment>
<file context>
@@ -115,27 +115,28 @@ do_uninstall() {
+ # Security check: ensure the path resolves inside the target directory.
+ # Use Python to compute a reliable relative path so symlinks cannot
+ # escape the boundary.
+ relative="$(python3 -c 'import os,sys; print(os.path.relpath(os.path.abspath(sys.argv[1]), sys.argv[2]))' "$full_path" "$codebuddy_root_resolved")"
+ case "$relative" in
+ ../*|..)
</file context>
Suggested change
| relative="$(python3 -c 'import os,sys; print(os.path.relpath(os.path.abspath(sys.argv[1]), sys.argv[2]))' "$full_path" "$codebuddy_root_resolved")" | |
| relative="$(python3 -c 'import os,sys; print(os.path.relpath(os.path.realpath(sys.argv[1]), sys.argv[2]))' "$full_path" "$codebuddy_root_resolved")" |
| // Security check: use path.relative() to ensure the manifest entry | ||
| // resolves inside the codebuddy directory. This is stricter than | ||
| // startsWith and correctly handles edge-cases with symlinks. | ||
| const relative = path.relative(codebuddyRootResolved, path.resolve(fullPath)); |
Contributor
There was a problem hiding this comment.
P1: Containment check uses path.resolve instead of resolving symlinks, allowing manifest entries that traverse a symlinked subdirectory to pass the check and delete files outside .codebuddy. This regresses the previous realpath-based safety and is security-relevant.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/uninstall.js, line 213:
<comment>Containment check uses path.resolve instead of resolving symlinks, allowing manifest entries that traverse a symlinked subdirectory to pass the check and delete files outside `.codebuddy`. This regresses the previous realpath-based safety and is security-relevant.</comment>
<file context>
@@ -211,27 +206,29 @@ async function doUninstall() {
+ // Security check: use path.relative() to ensure the manifest entry
+ // resolves inside the codebuddy directory. This is stricter than
+ // startsWith and correctly handles edge-cases with symlinks.
+ const relative = path.relative(codebuddyRootResolved, path.resolve(fullPath));
+ if (relative.startsWith('..') || path.isAbsolute(relative)) {
+ console.log(`Skipped: ${filePath} (outside target directory)`);
</file context>
Suggested change
| const relative = path.relative(codebuddyRootResolved, path.resolve(fullPath)); | |
| const resolvedFull = resolvePath(fullPath); | |
| const relative = path.relative(codebuddyRootResolved, resolvedFull); |
| return 1 | ||
| } | ||
|
|
||
| REPO_ROOT="$(find_repo_root)" |
Contributor
There was a problem hiding this comment.
P2: set -e causes the script to exit on a non-zero status from find_repo_root inside the command substitution, so the custom "Cannot locate" error block may never run. This makes root-not-found failures less diagnosable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/install.sh, line 33:
<comment>`set -e` causes the script to exit on a non-zero status from `find_repo_root` inside the command substitution, so the custom "Cannot locate" error block may never run. This makes root-not-found failures less diagnosable.</comment>
<file context>
@@ -13,9 +13,29 @@ set -euo pipefail
+ return 1
+}
+
+REPO_ROOT="$(find_repo_root)"
+if [ -z "$REPO_ROOT" ]; then
+ echo "Error: Cannot locate the ECC repository root."
</file context>
Suggested change
| REPO_ROOT="$(find_repo_root)" | |
| REPO_ROOT="$(find_repo_root || true)" |
| done < <(find "$REPO_ROOT/rules" -type f | sort) | ||
| fi | ||
|
|
||
| # Copy README files (skip install/uninstall scripts to avoid broken |
Contributor
There was a problem hiding this comment.
P2: Installer now skips copying uninstall.sh but still instructs users to run ./uninstall.sh in the target directory, so the documented uninstall path will not exist after install.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/install.sh, line 196:
<comment>Installer now skips copying uninstall.sh but still instructs users to run `./uninstall.sh` in the target directory, so the documented uninstall path will not exist after install.</comment>
<file context>
@@ -174,25 +193,13 @@ do_install() {
fi
- # Copy README files from this directory
+ # Copy README files (skip install/uninstall scripts to avoid broken
+ # path references when the copied script runs from the target directory)
for readme_file in "$SCRIPT_DIR/README.md" "$SCRIPT_DIR/README.zh-CN.md"; do
</file context>
| # file (VERSION). This keeps the script working even when it has been copied | ||
| # into a target project's .codebuddy/ directory. | ||
| find_repo_root() { | ||
| local dir="$(dirname "$SCRIPT_DIR")" |
Contributor
There was a problem hiding this comment.
P2: find_repo_root claims to walk up the directory tree but only checks the immediate parent of SCRIPT_DIR, so the “copied script” scenario described in comments will fail and abort installation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/install.sh, line 23:
<comment>find_repo_root claims to walk up the directory tree but only checks the immediate parent of SCRIPT_DIR, so the “copied script” scenario described in comments will fail and abort installation.</comment>
<file context>
@@ -13,9 +13,29 @@ set -euo pipefail
+# file (VERSION). This keeps the script working even when it has been copied
+# into a target project's .codebuddy/ directory.
+find_repo_root() {
+ local dir="$(dirname "$SCRIPT_DIR")"
+ # First try the parent of SCRIPT_DIR (original layout: .codebuddy/ lives in repo root)
+ if [ -f "$dir/VERSION" ] && [ -d "$dir/commands" ] && [ -d "$dir/agents" ]; then
</file context>
| } | ||
| } | ||
|
|
||
| // Copy README files (skip install/uninstall scripts to avoid broken |
Contributor
There was a problem hiding this comment.
P2: Installer no longer copies uninstall.js into the target directory but still tells users to run node uninstall.js there, so the uninstall instructions will fail on fresh installs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .codebuddy/install.js, line 270:
<comment>Installer no longer copies uninstall.js into the target directory but still tells users to run `node uninstall.js` there, so the uninstall instructions will fail on fresh installs.</comment>
<file context>
@@ -268,27 +267,14 @@ function doInstall() {
}
- // Copy README files
+ // Copy README files (skip install/uninstall scripts to avoid broken
+ // path references when the copied script runs from the target directory)
const readmeFiles = ['README.md', 'README.zh-CN.md'];
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.codebuddy/install.sh (1)
19-31:find_repo_rootcomment and behavior are inconsistent.The comment says “walking up,” but the function only checks one parent directory (Line 23-30). Either implement actual upward traversal or simplify the comment to match behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codebuddy/install.sh around lines 19 - 31, The comment for find_repo_root claims it “walk[s] up” from SCRIPT_DIR but the implementation only checks the immediate parent; update either the code or the comment. To fix, either implement upward traversal (loop walking parent directories from SCRIPT_DIR until root, checking for VERSION and commands/agents) inside the find_repo_root function, or change the comment to state it only checks the parent of SCRIPT_DIR; reference find_repo_root and SCRIPT_DIR to locate where to change behavior or wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codebuddy/install.js:
- Around line 186-188: The code unconditionally creates the legacy manifest
(.ecc-manifest) which can conflict with adapter-managed state; change the logic
around the manifest creation (the manifest constant and
ensureDir(path.dirname(manifest))) to first check for the presence of the
adapter-managed state file "ecc-install-state.json" in codebuddyFullPath and
skip creating the .ecc-manifest if that file exists; locate the manifest
declaration (const manifest = path.join(codebuddyFullPath, '.ecc-manifest')) and
the subsequent ensureDir call and wrap them in a conditional that only runs when
"ecc-install-state.json" is absent.
- Around line 270-302: The uninstall instructions are currently misleading
because uninstall.js is not copied into the installation directory; update
.codebuddy/install.js to also install the uninstaller: after the README copy
loop (which references readmeFiles and copyManagedFile) add a check for the
source uninstall.js (using scriptDir and codebuddyFullPath) and call
copyManagedFile(sourcePath, targetPath, manifest, 'uninstall.js') so the
uninstall script is present in the target directory and the later instruction to
run `node uninstall.js` will work.
In @.codebuddy/install.sh:
- Around line 196-227: The install summary refers to ./uninstall.sh but the
script isn't copied into the target directory; update the installer to copy the
uninstall script into $codebuddy_full_path and record it in the manifest: call
copy_managed_file for "$SCRIPT_DIR/uninstall.sh" ->
"$codebuddy_full_path/uninstall.sh" (similar to how README files are handled),
ensure it is marked executable (chmod +x) and ensure_manifest_entry is called
for ".uninstall.sh" or the appropriate manifest name so the uninstall command in
the summary will actually exist and be tracked.
In @.codebuddy/uninstall.sh:
- Around line 74-92: The uninstall fallback deletes the entire .codebuddy
directory when MANIFEST is missing, which can remove adapter-managed files like
ecc-install-state.json; update the branch that handles the missing MANIFEST (the
block referencing MANIFEST, CODEBUDDY_DIR and codebuddy_full_path) to first
check for the presence of ecc-install-state.json (or any adapter marker) and if
found do not perform a recursive rm -rf; instead abort or prompt explicitly that
adapter-managed content exists and require an additional confirmation before
deleting the whole directory, otherwise only remove known safe files or exit
without deleting.
---
Nitpick comments:
In @.codebuddy/install.sh:
- Around line 19-31: The comment for find_repo_root claims it “walk[s] up” from
SCRIPT_DIR but the implementation only checks the immediate parent; update
either the code or the comment. To fix, either implement upward traversal (loop
walking parent directories from SCRIPT_DIR until root, checking for VERSION and
commands/agents) inside the find_repo_root function, or change the comment to
state it only checks the parent of SCRIPT_DIR; reference find_repo_root and
SCRIPT_DIR to locate where to change behavior or wording.
🪄 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: 1db0ec66-8b31-4dbf-a033-cc8638c2e8b6
📒 Files selected for processing (6)
.codebuddy/README.zh-CN.md.codebuddy/install.js.codebuddy/install.sh.codebuddy/uninstall.js.codebuddy/uninstall.shtests/lib/install-targets.test.js
✅ Files skipped from review due to trivial changes (2)
- .codebuddy/README.zh-CN.md
- .codebuddy/uninstall.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/lib/install-targets.test.js
Comment on lines
+186
to
+188
| // Manifest file | ||
| const manifest = path.join(codebuddyFullPath, '.ecc-manifest'); | ||
| ensureDir(path.dirname(manifest)); |
Contributor
There was a problem hiding this comment.
Guard against adapter-managed state before creating legacy manifest state.
Line 187 initializes .ecc-manifest unconditionally. If ecc-install-state.json already exists (adapter-managed install), this script can mutate the same tree with a different state source, causing uninstall drift.
🔧 Proposed fix
// Manifest file
const manifest = path.join(codebuddyFullPath, '.ecc-manifest');
+ const adapterState = path.join(codebuddyFullPath, 'ecc-install-state.json');
+ if (fs.existsSync(adapterState)) {
+ throw new Error(
+ `Detected adapter-managed state (${adapterState}). Use the target-adapter install/uninstall flow instead of legacy .codebuddy/install.js.`
+ );
+ }
ensureDir(path.dirname(manifest));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codebuddy/install.js around lines 186 - 188, The code unconditionally
creates the legacy manifest (.ecc-manifest) which can conflict with
adapter-managed state; change the logic around the manifest creation (the
manifest constant and ensureDir(path.dirname(manifest))) to first check for the
presence of the adapter-managed state file "ecc-install-state.json" in
codebuddyFullPath and skip creating the .ecc-manifest if that file exists;
locate the manifest declaration (const manifest = path.join(codebuddyFullPath,
'.ecc-manifest')) and the subsequent ensureDir call and wrap them in a
conditional that only runs when "ecc-install-state.json" is absent.
Comment on lines
+270
to
+302
| // Copy README files (skip install/uninstall scripts to avoid broken | ||
| // path references when the copied script runs from the target directory) | ||
| const readmeFiles = ['README.md', 'README.zh-CN.md']; | ||
| for (const readmeFile of readmeFiles) { | ||
| const sourcePath = path.join(scriptDir, readmeFile); | ||
| if (fs.existsSync(sourcePath)) { | ||
| const targetPath = path.join(codebuddyFullPath, readmeFile); | ||
| copyManagedFile(sourcePath, targetPath, manifest, readmeFile); | ||
| } | ||
| } | ||
|
|
||
| // Add manifest itself | ||
| ensureManifestEntry(manifest, '.ecc-manifest'); | ||
|
|
||
| // Print summary | ||
| console.log('Installation complete!'); | ||
| console.log(''); | ||
| console.log('Components installed:'); | ||
| console.log(` Commands: ${commands}`); | ||
| console.log(` Agents: ${agents}`); | ||
| console.log(` Skills: ${skills}`); | ||
| console.log(` Rules: ${rules}`); | ||
| console.log(''); | ||
| console.log(`Directory: ${path.basename(codebuddyFullPath)}`); | ||
| console.log(''); | ||
| console.log('Next steps:'); | ||
| console.log(' 1. Open your project in CodeBuddy'); | ||
| console.log(' 2. Type / to see available commands'); | ||
| console.log(' 3. Enjoy the ECC workflows!'); | ||
| console.log(''); | ||
| console.log('To uninstall later:'); | ||
| console.log(` cd ${codebuddyFullPath}`); | ||
| console.log(' node uninstall.js'); |
Contributor
There was a problem hiding this comment.
Uninstall instructions are broken for fresh installs.
At Line 272-279, only README files are copied; no uninstall.js is installed. But Line 301-302 instructs users to run node uninstall.js inside the target directory, which will fail.
🔧 Proposed fix
- console.log(` cd ${codebuddyFullPath}`);
- console.log(' node uninstall.js');
+ console.log(` node "${path.join(scriptDir, 'uninstall.js')}" "${codebuddyFullPath}"`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codebuddy/install.js around lines 270 - 302, The uninstall instructions are
currently misleading because uninstall.js is not copied into the installation
directory; update .codebuddy/install.js to also install the uninstaller: after
the README copy loop (which references readmeFiles and copyManagedFile) add a
check for the source uninstall.js (using scriptDir and codebuddyFullPath) and
call copyManagedFile(sourcePath, targetPath, manifest, 'uninstall.js') so the
uninstall script is present in the target directory and the later instruction to
run `node uninstall.js` will work.
Comment on lines
+196
to
+227
| # Copy README files (skip install/uninstall scripts to avoid broken | ||
| # path references when the copied script runs from the target directory) | ||
| for readme_file in "$SCRIPT_DIR/README.md" "$SCRIPT_DIR/README.zh-CN.md"; do | ||
| if [ -f "$readme_file" ]; then | ||
| local_name=$(basename "$readme_file") | ||
| target_path="$codebuddy_full_path/$local_name" | ||
| copy_managed_file "$readme_file" "$target_path" "$MANIFEST" "$local_name" || true | ||
| fi | ||
| done | ||
|
|
||
| # Add manifest file itself to manifest | ||
| ensure_manifest_entry "$MANIFEST" ".ecc-manifest" | ||
|
|
||
| # Installation summary | ||
| echo "Installation complete!" | ||
| echo "" | ||
| echo "Components installed:" | ||
| echo " Commands: $commands" | ||
| echo " Agents: $agents" | ||
| echo " Skills: $skills" | ||
| echo " Rules: $rules" | ||
| echo "" | ||
| echo "Directory: $(basename "$codebuddy_full_path")" | ||
| echo "" | ||
| echo "Next steps:" | ||
| echo " 1. Open your project in CodeBuddy" | ||
| echo " 2. Type / to see available commands" | ||
| echo " 3. Enjoy the ECC workflows!" | ||
| echo "" | ||
| echo "To uninstall later:" | ||
| echo " cd $codebuddy_full_path" | ||
| echo " ./uninstall.sh" |
Contributor
There was a problem hiding this comment.
Uninstall command points to a script that is not installed.
Line 196-204 only copies README files, but Line 227 tells users to run ./uninstall.sh in the target .codebuddy directory. That command fails unless the file already exists there.
🔧 Proposed fix
- echo " cd $codebuddy_full_path"
- echo " ./uninstall.sh"
+ echo " bash \"$SCRIPT_DIR/uninstall.sh\" \"$codebuddy_full_path\""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codebuddy/install.sh around lines 196 - 227, The install summary refers to
./uninstall.sh but the script isn't copied into the target directory; update the
installer to copy the uninstall script into $codebuddy_full_path and record it
in the manifest: call copy_managed_file for "$SCRIPT_DIR/uninstall.sh" ->
"$codebuddy_full_path/uninstall.sh" (similar to how README files are handled),
ensure it is marked executable (chmod +x) and ensure_manifest_entry is called
for ".uninstall.sh" or the appropriate manifest name so the uninstall command in
the summary will actually exist and be tracked.
Comment on lines
+74
to
+92
| if [ ! -f "$MANIFEST" ]; then | ||
| echo "Warning: No manifest file found (.ecc-manifest)" | ||
| echo "" | ||
| echo "This could mean:" | ||
| echo " 1. ECC was installed with an older version without manifest support" | ||
| echo " 2. The manifest file was manually deleted" | ||
| echo "" | ||
| read -p "Do you want to remove the entire $CODEBUDDY_DIR directory? (y/N) " -n 1 -r | ||
| echo | ||
| if [[ ! $REPLY =~ ^[Yy]$ ]]; then | ||
| echo "Uninstall cancelled." | ||
| exit 0 | ||
| fi | ||
| rm -rf "$codebuddy_full_path" | ||
| echo "Uninstall complete!" | ||
| echo "" | ||
| echo "Removed: $codebuddy_full_path/" | ||
| exit 0 | ||
| fi |
Contributor
There was a problem hiding this comment.
Manifest-missing fallback can delete adapter-managed content.
When .ecc-manifest is absent, Line 87 can recursively delete the whole .codebuddy directory after prompt. This is unsafe when ecc-install-state.json exists (adapter flow) or mixed content is present.
🔧 Proposed fix
if [ ! -f "$MANIFEST" ]; then
+ if [ -f "$codebuddy_full_path/ecc-install-state.json" ]; then
+ echo "Error: Detected adapter-managed install state (ecc-install-state.json)."
+ echo "Use the install-target adapter uninstall flow instead of legacy uninstall.sh."
+ exit 1
+ fi
echo "Warning: No manifest file found (.ecc-manifest)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codebuddy/uninstall.sh around lines 74 - 92, The uninstall fallback deletes
the entire .codebuddy directory when MANIFEST is missing, which can remove
adapter-managed files like ecc-install-state.json; update the branch that
handles the missing MANIFEST (the block referencing MANIFEST, CODEBUDDY_DIR and
codebuddy_full_path) to first check for the presence of ecc-install-state.json
(or any adapter marker) and if found do not perform a recursive rm -rf; instead
abort or prompt explicitly that adapter-managed content exists and require an
additional confirmation before deleting the whole directory, otherwise only
remove known safe files or exit without deleting.
peiking88
pushed a commit
to peiking88/everything-claude-code
that referenced
this pull request
Apr 4, 2026
…ripts (affaan-m#1038) * feat(install): add CodeBuddy(Tencent) adaptation with installation scripts * fix: add codebuddy to SUPPORTED_INSTALL_TARGETS * fix(codebuddy): resolve installer path issues, unused vars, and uninstall safety
This was referenced Apr 5, 2026
This was referenced May 11, 2026
FrancescoRosciano
pushed a commit
to FRosciano-Mambo/everything-claude-code
that referenced
this pull request
Jun 1, 2026
…ripts (affaan-m#1038) * feat(install): add CodeBuddy(Tencent) adaptation with installation scripts * fix: add codebuddy to SUPPORTED_INSTALL_TARGETS * fix(codebuddy): resolve installer path issues, unused vars, and uninstall safety
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integrate CodeBuddy IDE into the existing Target Adapter install architecture,
replacing standalone shell/Node.js scripts with a proper adapter that
supports the full install lifecycle (install/doctor/repair/uninstall).
CodeBuddy CLI is fully compatible with the Claude Code ecosystem — it supports
the same hooks (9 events), agents (YAML frontmatter), commands, skills, and
rules directory structure under
.codebuddy/. This PR adds a project-scopedadapter and registers it across all applicable modules.
Key decisions:
rules-core,agents-core,commands-core,hooks-runtime,and all skill modules — CodeBuddy supports all of these natively
platform-configs(paths reference.cursor/,.codex/,.opencode/which are other IDE configs),orchestration(tmux/worktreescripts, CLI-specific)
.tencentcodebuddy/directory — identical duplicate of.codebuddy/with only variable names changed
Type
Changes
New files:
scripts/lib/install-targets/codebuddy-project.js— Project-scoped adapter(~47 lines, mirrors
cursor-project.jswith.codebuddy/root and flat ruleoperations)
Modified files:
scripts/lib/install-targets/registry.js— Registercodebuddy-projectadapter
manifests/install-modules.json— Add"codebuddy"target to 17 applicablemodules (all except
platform-configsandorchestration)schemas/install-modules.schema.json— Add"codebuddy"to allowed targetenum
schemas/ecc-install-config.schema.json— Add"codebuddy"to configtarget enum
tests/lib/install-targets.test.js— Add 4 new tests (resolveRoot, supportslookup, flat rules planning, validate/planOperations)
.codebuddy/README.md+.codebuddy/README.zh-CN.md— Updated to documentthe recommended adapter-based install workflow
Testing
Checklist
Summary by cubic
Adds a CodeBuddy (
codebuddy) install target with a project adapter and cross‑platform install/uninstall scripts to bring ECC’s install/doctor/repair/uninstall flow to Tencent CodeBuddy. Also fixes installer path handling and hardens uninstall safety with manifest-only deletion and directory boundary checks.New Features
codebuddy-projectadapter with.codebuddy/root and flat rules planning; registered in the install targetregistry.codebuddyinmanifests/install-modules.json, schemas, andSUPPORTED_INSTALL_TARGETS..codebuddy/install.{js,sh}and.codebuddy/uninstall.{js,sh}plus English/Chinese docs; tests cover adapter lookup, planning, and validation.Bug Fixes
.codebuddy/directory; skip copying installer scripts to targets to avoid broken paths.SUPPORTED_INSTALL_TARGETSin sync.Written for commit 15e6e98. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores