Repository navigation
Conversation
|
|
|
Hi maintainers 👋 Just a quick update — I've refactored the CodeBuddy/Claude settings.json handling to eliminate the duplication I introduced earlier. The shared logic is now extracted into:
CodeBuddy support is now fully wired up and production-ready:
All 1996 tests pass. The refactoring makes it straightforward to add future agents that share the same I'd love to get this merged so CodeBuddy Code users can benefit from RTK's token savings. Happy to address any feedback. Thanks for reviewing! |
|
When will it be released? |
|
Hi @FlorianBruniaux @pszymkowiak — gentle ping on this PR. It has been open for about a month and all 1996 tests pass with zero conflicts. Just a bit of context on why this matters: I am actively working on adding Headroom support for CodeBuddy Code, and that project integrates RTK as its token-optimization layer. Having this PR merged would make the Headroom + RTK + CodeBuddy Code stack work seamlessly out of the box, which benefits both projects. Quick recap of the change
Scope
Would really appreciate a review when you have a moment. Happy to address any feedback or make adjustments. Thanks! |
|
Hi @aeppling @KuSh — gentle ping on this PR. I noticed you two have been actively merging PRs this past week (e.g. #2514, #2465, #2416, #2406, #2394, #2294). Would you mind taking a look at this one when you have a moment? Quick recap
This has been open since May 19. Happy to address any feedback or make changes. Thanks! |
KuSh
left a comment
KuSh
left a comment
There was a problem hiding this comment.
Hi, thanks for the PR. A few changes are needed before we can consider merging this.
I’m not familiar with CodeBuddy Code, and while we plan to support the main tools, we don’t necessarily want to add every niche one. Especially since this appears to be compatible with Claude, it seems a symlink to the .claude directory might be all that’s needed.
Could you share a bit more context on why CodeBuddy Code should be integrated here, such as what unique needs it covers beyond Claude compatibility, and how significant its user base or adoption is?
- Delegate run_codebuddy to run_claude (zero protocol duplication) - Switch remove_hooks_from_settings_at to single &str (YAGNI per reviewer) - Simplify Skip-mode match to unified print_manual_instructions on Skipped|Declined - Restore Claude-specific output (backup hint + Restart) lost during refactor - Fix stale doc comment on patch_settings_json_at (relationship reversed) - Extract uninstall_codebuddy_at for testability + add 3 unit tests - Simplify resolve_codebuddy_dir match to ? - Merge Patched|WouldPatch no-op arms; gate Restart print on AlreadyPresent - Extract patch_mode_from_flags helper (dedup gemini/codebuddy/default) - Move CodeBuddy entries to list ends per reviewer nitpick - Add protocol-stability note to hooks/codebuddy/README.md - Shared rtk_awareness_for_agent template; drop codebuddy/rtk-awareness.md All 1999 tests pass.
|
Hi @KuSh, thanks for the thorough review — all feedback addressed in the latest push (commit On the top-level question: why CodeBuddy Code, not a symlinkCodeBuddy Code is an independent product (hosted at cnb.cool/codebuddy/codebuddy-code, built by Tencent) — it is not a Claude Code fork or wrapper. A symlink from
The only thing CodeBuddy Code shares with Claude Code is the If you'd like more context on adoption: CodeBuddy Code is the AI coding agent bundled with CodeBuddy (Tencent's AI dev suite). I'm actively integrating RTK as the token-optimization layer for CodeBuddy Code via Headroom, so this PR is what makes that stack work out of the box. Inline comments
Happy to adjust anything else. |
KuSh
left a comment
KuSh
left a comment
There was a problem hiding this comment.
Thanks for the changes! Unfortunately, there’s one big blocker: CodeBuddy handles exit codes differently from Claude, so we’ll need to adapt the preToolUse shell to account for that. Otherwise LGTM!
|
Thanks for catching the exit-code/protocol blocker (@KuSh). I dug into it and the root cause is a protocol field-name mismatch rather than exit codes themselves: CodeBuddy's PreToolUse expects Changes in the latest push (
On the earlier inline suggestion to collapse Verified locally: Regarding the |
|
All code changes have been updated as requested. Ready for merge. Thanks for the review! |
|
Hi @KuSh, thanks for the thorough review on this PR. All your feedback has been addressed:
All 2448 tests pass (including the 3 new CodeBuddy unit tests). I believe this is ready for another review — would you be able to take a look? Thanks! |
|
@TaKO8Ki pls merge this PR |
|
Could you please resolve the conflicts? @studyzy |
Squashed from 9 commits on fork develop branch: - feat: add CodeBuddy Code support - refactor: deduplicate Claude/CodeBuddy settings.json hook logic - refactor: address PR rtk-ai#1967 review feedback - docs: rename CodeBuddy Code -> CodeBuddy and update official URL - fix(codebuddy): emit correct protocol fields and deny decision - fix(codebuddy): differentiate audit_log action for allow vs ask - fix(codebuddy): add nosemgrep annotation for filesystem-deletion - fix: make CODEBUDDY.md use @RTK.md reference pattern - feat(codebuddy): use host permission settings and preserve tool_input - fix(codebuddy): emit updatedInput instead of modifiedInput in PreToolUse Rebased onto offical/develop (50 new upstream commits integrated). Conflicts resolved: both Droid (upstream) and CodeBuddy (fork) integrations kept.
|
Conflicts resolved and force-pushed. Verified locally: Key changes in the latest push:
Ready for review and merge. Thanks! |
KuSh
left a comment
KuSh
left a comment
There was a problem hiding this comment.
Not at home, so hard to review on phone but here are some preliminary feedback of your new proposition
|
Thanks for the review, @KuSh! All feedback addressed in the latest push (commit 72b3611). Summary of changes:
All 2578 tests pass. |
KuSh
left a comment
KuSh
left a comment
There was a problem hiding this comment.
My main concern is the auto-execute bypass. I couldn’t find anything referencing the issue, and I’d prefer to know it’s tracked and temporary. We could then hold off on this PR until it’s fixed.
In the meantime, there are still a few bugs lurking, along with some code duplication and minor suggestions. It would also be great if you could rebase, since you have conflicts right now
86d729c to
d37c128
Compare
studyzy
left a comment
studyzy
left a comment
There was a problem hiding this comment.
Thanks for the thorough review. I've addressed the auto-execute bypass concern — this was the main blocker.
Auto-execute bypass is resolved. The earlier version collapsed AskRewrite into "allow" as a workaround for a CodeBuddy IDE bug where permissionDecision: "ask" popped a confirmation but ignored the rewritten command. That bypass is now removed: AskRewrite maps back to permissionDecision: "ask" and forwards the rewrite via updatedInput, exactly like the allow path. The ask/allow distinction is honored again.
On the IDE bug itself: it's tracked upstream with CodeBuddy and is scheduled to be fixed in their next release. The workaround (and the revert path) is documented in hooks/codebuddy/README.md and src/hooks/hook_cmd.rs with WORKAROUND/TODO labels, so once the fix lands we simply restore the mapping — no protocol change needed on our side.
Rebased. The branch is now a single clean commit on top of the latest rtk-ai/develop, and the PR is mergeable with no conflicts.
Code duplication / minor suggestions. I addressed the ones that were in scope:
build_updated_inputis now shared by the Claude, CodeBuddy, and Droid paths (no duplication).- The manual-install snippet and auto-install matcher were aligned to
Bash|execute_command. find_project_rootnow takes the caller's ownagent_dir, so each host resolves its config independently — a sibling host's dir in a subdirectory no longer affects another host's rules.
Two items I deliberately kept out of scope to keep the change minimal, happy to follow up on either:
load_permission_rules(Claude) andload_codebuddy_rulesshare shape but are kept separate to avoid touching the long-stable Claude path; I can extract a shared helper if you'd like.- Consolidating
remove_rtk_reference_from_agentsandremove_rtk_refs_from_mdinto one host-agnostic helper (the CodeBuddy path already uses the_from_mdhelper, which uses atomic writes and exact matching).
Happy to adjust anything else. Thanks again for the careful review!
Closes rtk-ai#1966 Adds first-class support for CodeBuddy (Tencent Cloud AI Code Editor) via its PreToolUse hook protocol, which mirrors Claude Code's `tool_name` + `tool_input.command` JSON shape. Changes: - `rtk hook codebuddy` — PreToolUse hook processor (reads JSON from stdin, rewrites `tool_input.command`) - `rtk init -g --agent codebuddy` — installs the hook into `~/.codebuddy/settings.json` and writes `~/.codebuddy/CODEBUDDY.md` - `rtk init -g --agent codebuddy --uninstall` — removes all RTK artifacts - `Host::CodeBuddy` permission rules, read from `.codebuddy/settings.json` and `.codebuddy/settings.local.json` The hook emits `updatedInput` (CodeBuddy upstream converged on this field, same as Claude Code) and mirrors RTK's verdict: AllowRewrite -> "allow", AskRewrite -> "ask". Shared settings.json helpers (`patch_settings_json_at`, `remove_hooks_from_settings_at`, `rtk_awareness_for_agent`) are reused so no Claude/CodeBuddy logic is duplicated.
|
Quick clarification on two protocol points, based on direct communication with the CodeBuddy team:
Given both points, the current implementation (emit |
KuSh
left a comment
KuSh
left a comment
There was a problem hiding this comment.
LGTM, except for a few nitpicks. Thanks for all your work and dedication. I’ll hold off on merging until you can confirm the ask bug is fixed in CodeBuddy though
| } else { | ||
| hooks::init::PatchMode::Ask | ||
| }; | ||
| let patch_mode = hooks::init::patch_mode_from_flags(auto_patch, no_patch); |
There was a problem hiding this comment.
Nitpick: patch_mode_from_flags seems to be used only here, so it doesn’t need to be public in init
There was a problem hiding this comment.
patch_mode_from_flags lives in src/hooks/init.rs but is called from src/main.rs in three places (the CodeBuddy branch, the default Claude/Copilot branch, and another agent branch), so it must remain pub — it's a cross-module API, not an internal-only helper. No change needed here.
There was a problem hiding this comment.
Call count wasn't the objection — it's which module calls it.
patch_mode_from_flags is defined in src/hooks/init.rs:98, but init.rs itself never calls it. All three call sites are in src/main.rs (2046, 2087, 2096). So the pub exists purely to export a helper out of a module that has no use for it.
It's also introduced by this PR — it isn't present at the merge-base (9936b2b) — so it's not pre-existing debt.
And the dedup is incomplete either way: the Vibe branch at src/main.rs:2077-2083 still spells out the exact if/else the helper was extracted to replace, four lines above a patch_mode_from_flags(...) call:
} else if agent == Some(AgentTarget::Vibe) {
let patch_mode = if auto_patch {
hooks::init::PatchMode::Auto
} else if no_patch {
hooks::init::PatchMode::Skip
} else {
hooks::init::PatchMode::Ask
};Suggestion: move it into main.rs as a private fn patch_mode_from_flags and use it for the Vibe branch too. That drops a pub item from init's API surface and removes the leftover inline copy in one go — main.rs already names PatchMode::{Auto,Skip,Ask} directly, so nothing new needs exporting.
- Generalize patch_claude_md into patch_md_with_ref so verbose/dry-run messages no longer hardcode CLAUDE.md when operating on CODEBUDDY.md - Interpolate CODEBUDDY_MATCHER / CODEBUDDY_HOOK_COMMAND in the manual install snippet instead of hardcoding the strings - Validate tool_name in run_codebuddy via codebuddy_execute_command, mirroring droid_execute_command - Extract load_rules_from_paths shared by load_permission_rules (Claude) and load_codebuddy_rules, which differ only in resolved settings paths
|
@KuSh thanks for the thorough review — all remaining nits are addressed in the latest push ( Confirming the
|
KuSh
left a comment
KuSh
left a comment
There was a problem hiding this comment.
Re-reviewed at 1e23b66. First: I verified all four fixes from your last push are real, not just described — patch_md_with_ref interpolates md_name in all five messages, the manual-install snippet uses the constants, codebuddy_execute_command faithfully mirrors droid_execute_command, and load_rules_from_paths collapses the duplication. cargo fmt --check, cargo clippy --all-targets -- -D warnings and cargo test --all (2662 tests) are all green on the head. Thanks for the thorough turnaround.
The following are new findings from a fresh pass over the whole PR, not leftovers from the previous round. One is significant (the ask verdict on default), the rest are polish. Leaving this as a comment rather than blocking — your call on scope.
Separately, the patch_mode_from_flags thread is still open; I replied there with why the visibility point stands.
| HookDecision::AskRewrite(r) => { | ||
| audit_log("ask", cmd, &r); | ||
| ("ask", "RTK auto-rewrite", Some(r)) | ||
| } |
There was a problem hiding this comment.
This makes CodeBuddy prompt on every rewritten command.
decide_from_verdict maps both an explicit Ask rule and the Default "no rule matched" verdict to HookDecision::AskRewrite:
// hook_cmd.rs:266-270
match get_rewritten(cmd) {
Some(r) if verdict == PermissionVerdict::Allow => HookDecision::AllowRewrite(r),
Some(r) => HookDecision::AskRewrite(r), // <- Default lands here too
None => HookDecision::Defer,
}So this arm asserts permissionDecision: "ask" for the default case. And the default case is the normal case: patch_settings_json_at writes a settings.json containing only a hooks key — no permissions block — so after a fresh rtk init -g --agent codebuddy, load_codebuddy_rules() returns empty vectors and every command RTK rewrites gets "ask". The agent runs git status, RTK rewrites it, CodeBuddy shows a blocking approval dialog. Every time.
This is the regression already documented in this file at hook_cmd.rs:288-295:
permissionDecision: "allow"is only ever asserted for an explicit, user-configured Allow rule. Every other rewrite (Default verdict or an explicit Ask rule) omits the field entirely, leaving the host's own native prompt/allowlist flow in control — see #3037, where asserting"ask"here made Copilot CLI 1.0.66+ force a blocking dialog with no "remember" option on every rewritten command.
Both sibling paths follow that rule and insert the field only when the verdict is an explicit Allow — process_claude_payload (:598/:613) and vscode_response_from_decision (:304/:314). CodeBuddy is currently the only host that asserts "ask".
I realise honouring the ask/allow distinction was deliberate, and now that the IDE bug is fixed the rewrite does survive the prompt. But #3037 was about prompt fatigue, not correctness — and that argument applies here unchanged.
Two ways out:
- Omit
permissionDecisionunless the verdict is an explicit Allow (mirrors Claude/VS Code exactly), or - Split
DefaultfromAskinHookDecisionso only a user-configured Ask rule produces"ask".
The second preserves your intent while keeping the default install quiet.
| fn codebuddy_execute_command(v: &Value) -> Option<&str> { | ||
| let tool_name = v.get("tool_name").and_then(|t| t.as_str()).unwrap_or(""); | ||
| if !matches!(tool_name, "Bash" | "execute_command" | "") { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
This gating has no test coverage.
The function it mirrors is tested for exactly this behaviour:
test_droid_ignores_non_execute_tool(hook_cmd.rs:2194)test_droid_bash_tool_name_accepted_defensively(hook_cmd.rs:2205)
The four CodeBuddy tests (:1677-:1728) all construct their payload with codebuddy_payload() and call codebuddy_response_from_decision directly, so none of them ever reach codebuddy_execute_command. If a later refactor drops or inverts the matches! guard, nothing fails and RTK silently starts rewriting commands for non-shell tools again.
Two small tests mirroring the Droid pair would close it — a payload with tool_name: "Read" returning None, and one with execute_command / missing tool_name returning the command.
| if dry_run { | ||
| print_dry_run_footer(); | ||
| } else { | ||
| println!("\nCodeBuddy hook installed (global).\n"); |
There was a problem hiding this comment.
Reports success even when nothing was installed.
This println! is unconditional and runs before the match result below, so it fires on PatchResult::Declined and PatchResult::Skipped too.
Answer n at the consent prompt (or pass --no-patch) and the output reads:
CodeBuddy hook installed (global).
To add manually, patch ~/.codebuddy/settings.json:
{"hooks": {"PreToolUse": [...]}}
Two contradictory statements, success first. Anyone skimming believes it worked, while settings.json was never touched and RTK stays inert on every subsequent command.
The Claude path gates the equivalent text on the result (init.rs:964):
if result == PatchResult::Patched {
...
}Same gate here would fix it.
| PatchResult::Patched | PatchResult::WouldPatch => {} | ||
| PatchResult::AlreadyPresent => { | ||
| println!(" settings.json: hook already present"); | ||
| println!(" Restart CodeBuddy. Test with: git status\n"); |
There was a problem hiding this comment.
The restart notice is on the wrong arm.
AlreadyPresent — the case where nothing changed — tells the user to restart. Patched — the case where the hook was just written and a restart is exactly what's needed — is an empty block and prints nothing.
Net effect on a first install: RTK writes the hook, says nothing about restarting, the user doesn't restart, the hook never loads, and RTK appears broken. Run it a second time and then you're told to restart.
The Claude path attaches the notice to Patched (init.rs:970-976); worth matching.
| fn rtk_awareness_for_agent(agent_name: &str, md_filename: &str) -> String { | ||
| RTK_SLIM | ||
| .replace("Claude Code", agent_name) | ||
| .replace("CLAUDE.md", md_filename) | ||
| } |
There was a problem hiding this comment.
This blind replace puts a false instruction into ~/.codebuddy/RTK.md.
.replace("Claude Code", agent_name) also rewrites line 10 of hooks/claude/rtk-awareness.md:
rtk discover # Analyze Claude Code history for missed opportunities
which becomes "Analyze CodeBuddy history for missed opportunities". But rtk discover can't do that — DiscoverProvider::projects_dir (src/discover/provider.rs:46-48) resolves only claude_dir.join("projects"), i.e. ~/.claude/projects. A CodeBuddy-only user follows the instructions RTK just installed for them and gets an empty result with no explanation.
Simplest fix is to drop that line from the non-Claude variant, or leave rtk discover's description untouched by the substitution.
| fn find_project_root(agent_dir: &str) -> Option<PathBuf> { | ||
| // Fast path: walk up CWD looking for the agent's config dir — no subprocess needed. | ||
| let mut dir = std::env::current_dir().ok()?; | ||
| loop { | ||
| if dir.join(CLAUDE_DIR).exists() { | ||
| if dir.join(agent_dir).exists() { | ||
| return Some(dir); | ||
| } | ||
| if !dir.pop() { |
There was a problem hiding this comment.
Parameterising this changes behaviour for Gemini and Droid, not just CodeBuddy.
At the merge-base this walked up looking for .claude/ unconditionally. This PR also repoints the existing callers — gemini_settings() now passes GEMINI_DIR, droid_settings_scopes() passes DROID_DIR.
.claude/ is present in most projects, so the fast walk almost always hit before. .gemini/ / .droid/ often aren't, so those paths now fall through to the git rev-parse --show-toplevel fallback below — a subprocess spawn per intercepted Bash command, on the hook hot path, against the repo's documented <10 ms startup budget. It only bites when CWD is outside $HOME (inside $HOME the walk finds ~/.gemini), so the blast radius is narrow, but it's a real change to paths this PR isn't otherwise touching.
Caching the resolved root, or keeping .claude/ as a secondary probe before falling back to git, would avoid it.
For the record on the neighbouring concern: the double-push of the two home paths that get_codebuddy_settings_paths can produce when the walk resolves the root to $HOME is not a new bug — get_settings_paths (permissions.rs:156) has had the identical shape for Claude all along. It costs a redundant read and duplicate rules, never a wrong verdict.
|
This narrows a match used by the existing Claude uninstall path, not just new CodeBuddy code — worth flagging as a regression.
.filter(|line| {
let trimmed = line.trim();
!refs.contains(&trimmed)
})The old code it replaces used a prefix match: .filter(|line| !line.trim().starts_with(RTK_MD_REF))
Repro: a |
`remove_rtk_refs_from_md` replaced the previous prefix match (`starts_with(RTK_MD_REF)`) with an exact line match, so a reference line carrying a trailing comment (e.g. `@RTK.md — see below`) was left behind on uninstall. Restore substring detection + prefix stripping and add regression tests.
|
@pszymkowiak good catch — you're right, that's a regression in the existing Claude uninstall path, not just a gap in the new CodeBuddy code. Fixed in the latest push (
Verified: I deliberately left |
|
@studyzy @KuSh I would be happy to help move this PR forward after #4151 lands. I verified |
|
Thanks @yeahjack! Yes, absolutely open to a follow-up patch on this branch. Once #4151 lands, I'll coordinate the rebase onto the shared hook helpers, and your help with the remaining default/ask behavior and regression tests would be very welcome — especially given your verified testing on CodeBuddy CLI 2.151.0. Happy to add you as a collaborator on my fork so you can push follow-up patches directly; will follow up on that. Thanks for offering to help move this forward! |
|
@studyzy A gentle follow-up: could you let me know when the rebase is ready and send the collaborator invite? I can then help with the default/ask behavior and regression tests as discussed. Thanks! |
8 participants
Closes #1966
Summary
Adds first-class support for CodeBuddy Code, a Claude-powered AI coding assistant whose config lives in
.codebuddy/.CodeBuddy Code uses the same
PreToolUsehook JSON protocol as Claude Code, so the hook implementation delegates directly to the existingprocess_claude_payloadlogic.Changes
New commands
rtk hook codebuddy— PreToolUse hook processor (reads JSON from stdin, rewritestool_input.command)rtk init -g --agent codebuddy— installs hook into~/.codebuddy/settings.jsonand writes~/.codebuddy/CODEBUDDY.mdrtk init -g --agent codebuddy --uninstall— removes all RTK artifactsFiles changed
src/hooks/constants.rsCODEBUDDY_DIR(.codebuddy) andCODEBUDDY_HOOK_COMMANDconstantssrc/hooks/hook_cmd.rsrun_codebuddy()— delegates toprocess_claude_payloadsrc/hooks/init.rsrun_codebuddy_mode(),uninstall_codebuddy(), andpatch_settings_json_at()helpersrc/main.rsAgentTarget::Codebuddy,HookCommands::Codebuddy, dispatch logichooks/codebuddy/rtk-awareness.md~/.codebuddy/CODEBUDDY.mdhooks/codebuddy/README.mdDesign notes
PreToolUse+tool_input.commandJSON format as Claude Code, sorun_codebuddy()simply callsprocess_claude_payload().patch_settings_json_at(): Extracted a new helper that patchessettings.jsonin an arbitrary directory (instead of hardcoding the Claude config dir). This makes it easy to add future agents with similar config layouts.~/.codebuddy/).Testing