fix(runtime): handle RTK missing hook - #1539
Conversation
📝 WalkthroughWalkthroughIntroduces a typed classification function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/main/lib/agentRuntime/rtkRuntimeService.test.ts (1)
178-292: Tests faithfully exercise the new classifier branch at both layers.Both
prepareShellCommandandstartHealthCheckare covered for the code=3 + "No hook installed" path, and the health-check assertion that therewriteinvocation was actually issued is a nice belt-and-braces check.Optional follow-up: consider one negative test where
code=3is paired with a non-matching stderr (e.g. a generic error) to lock down that this only opens the gate for the documented missing-hook signal and doesn't accidentally treat unrelatedcode=3failures as rewrites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/lib/agentRuntime/rtkRuntimeService.test.ts` around lines 178 - 292, Add a negative test case to ensure we only treat exit code 3 as "missing global hook" when stderr matches the documented message: create a test (mirroring the existing positive ones) that uses prepareShellCommand and startHealthCheck with a mocked runCommand returning code: 3 but stderr: 'Some other error' (or similar), then assert that the command is NOT rewritten (result.rewritten false, rtkApplied false, rtkMode 'bypass' or appropriate fallback) and that startHealthCheck does not count this as a rewrite invocation; reference prepareShellCommand and startHealthCheck to locate where the classifier behavior is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/lib/agentRuntime/rtkRuntimeService.ts`:
- Around line 120-149: Replace the magic numbers and inline regex in
classifyRtkRewriteResult with named SCREAMING_SNAKE_CASE constants (e.g.
RTK_EXIT_SUCCESS, RTK_EXIT_BYPASS, RTK_EXIT_DENY, RTK_EXIT_NO_HOOK and
NO_HOOK_REGEX) and update the logic so that exit code 2 (RTK_EXIT_DENY) returns
the same bypass result as exit code 1; keep the existing special-case that
treats RTK_EXIT_NO_HOOK + "No hook installed" as a rewritten result but add a
short comment explaining why an otherwise-"ask" exit code is treated as
"rewritten". Ensure you update the checks in classifyRtkRewriteResult to use
these constants (and the NO_HOOK_REGEX) instead of hardcoded numbers/strings.
---
Nitpick comments:
In `@test/main/lib/agentRuntime/rtkRuntimeService.test.ts`:
- Around line 178-292: Add a negative test case to ensure we only treat exit
code 3 as "missing global hook" when stderr matches the documented message:
create a test (mirroring the existing positive ones) that uses
prepareShellCommand and startHealthCheck with a mocked runCommand returning
code: 3 but stderr: 'Some other error' (or similar), then assert that the
command is NOT rewritten (result.rewritten false, rtkApplied false, rtkMode
'bypass' or appropriate fallback) and that startHealthCheck does not count this
as a rewrite invocation; reference prepareShellCommand and startHealthCheck to
locate where the classifier behavior is exercised.
🪄 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: d017d806-3fbd-4611-992c-6342c8605789
📒 Files selected for processing (2)
src/main/lib/agentRuntime/rtkRuntimeService.tstest/main/lib/agentRuntime/rtkRuntimeService.test.ts
| function classifyRtkRewriteResult(result: CommandResult): RtkRewriteResult { | ||
| const stdout = result.stdout.trim() | ||
| const stderr = result.stderr.trim() | ||
|
|
||
| if (result.code === 0 && stdout) { | ||
| return { | ||
| status: 'rewritten', | ||
| command: stdout | ||
| } | ||
| } | ||
|
|
||
| if (result.code === 3 && stdout && /No hook installed/i.test(stderr)) { | ||
| return { | ||
| status: 'rewritten', | ||
| command: stdout | ||
| } | ||
| } | ||
|
|
||
| if (result.code === 1) { | ||
| return { | ||
| status: 'bypass', | ||
| message: 'RTK rewrite did not match this command' | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| status: 'failure', | ||
| message: stderr || stdout || 'rtk rewrite failed' | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
rtk CLI rewrite command exit code conventions
💡 Result:
For rtk rewrite, the documented exit-code “conventions” (used by the Claude Code hook rewrite script / protocol) are: - Exit code 0: A rewrite was found and the command is auto-allowed. (Non deny/ask rules matched; hook should apply rewrite without requiring user approval.) - Exit code 1: No RTK equivalent exists for the command; pass through unchanged. - Exit code 2: A deny rule matched; pass through (do not apply rewrite). - Exit code 3: An “ask” rule matched; rewrite the command but let the host (e.g., Claude Code) prompt the user for approval. Important practical note: several Cursor/Claude Code hook issues show that if a hook treats any non-zero exit as failure (e.g., using ||), then rewrites with exit code 3 can be incorrectly discarded, and the hook may return {} instead of applying the rewritten command. Also, at least one issue indicates exit code 3 for read-only commands (ls/grep/find/wc/cat/tail) caused silent drops in permissive Claude Code sessions, leading to a recommendation/implementation to classify those as exit code 0 (auto-allow) instead—while keeping exit code 3 for state-modifying commands requiring an “ask”/approval flow.
Citations:
- 1: Read-only commands (ls/grep/find/wc/cat/tail) should return exit 0 from
rtk rewrite, not exit 3 rtk-ai/rtk#1232 - 2: preToolUse hook broken: rtk rewrite exits code 3 on success, || fallback swallows all rewrites rtk-ai/rtk#1145
- 3: Exit 3
updatedInputmutations silently dropped in permissive Claude Code Bash modes rtk-ai/rtk#1233 - 4: Cursor hook: rtk rewrite success exit code causes rewrites to be dropped; optional PATH note for Linux rtk-ai/rtk#1272
🏁 Script executed:
# First, read the file to understand context and see type definitions
head -n 130 src/main/lib/agentRuntime/rtkRuntimeService.ts | tail -n 70Repository: ThinkInAIXYZ/deepchat
Length of output: 1845
🏁 Script executed:
# Check earlier in file for type definitions and any existing constants
head -n 70 src/main/lib/agentRuntime/rtkRuntimeService.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 1794
🏁 Script executed:
# Search for other references to RTK exit codes or CommandResult in the codebase
rg "result\.code" --type ts --type tsx -B2 -A2Repository: ThinkInAIXYZ/deepchat
Length of output: 93
🏁 Script executed:
# Read the exact lines mentioned in the review (120-149)
sed -n '120,149p' src/main/lib/agentRuntime/rtkRuntimeService.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 701
🏁 Script executed:
# Search for any other references or usages of this function or exit codes
rg "classifyRtkRewriteResult\|exit.*code\|result\.code" --type ts -B1 -A1Repository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Check if there are any comments or documentation about RTK exit codes
rg "exit.*code|RTK.*code|rewrite.*result" --type ts | head -20Repository: ThinkInAIXYZ/deepchat
Length of output: 1610
🏁 Script executed:
# Check where this function is called and how the result is used
rg "classifyRtkRewriteResult" --type ts -B2 -A5Repository: ThinkInAIXYZ/deepchat
Length of output: 1958
🏁 Script executed:
# Look for any comments or documentation about the "No hook installed" pattern
rg "No hook installed\|hook.*installed" --type ts -B3 -A3Repository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Check if there's any handling for exit code 2 elsewhere
rg "code.*2\|exit.*2" src/main/lib/agentRuntime/rtkRuntimeService.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 47
Extract RTK exit-code constants and handle exit code 2 (deny rule).
The classifier hardcodes exit codes (0, 1, 3) and the "No hook installed" pattern inline. Per coding guidelines, these should be named constants using SCREAMING_SNAKE_CASE. More importantly, exit code 2 (deny rule matched) currently falls through to failure, but should map to bypass per RTK documentation—a deny rule means "do not apply rewrite," which aligns with bypass semantics.
Suggested fix
+const RTK_EXIT_CODE_REWRITTEN = 0
+const RTK_EXIT_CODE_BYPASS = 1
+const RTK_EXIT_CODE_DENY = 2
+const RTK_EXIT_CODE_MISSING_HOOK = 3
+const RTK_MISSING_HOOK_PATTERN = /No hook installed/i
+
function classifyRtkRewriteResult(result: CommandResult): RtkRewriteResult {
const stdout = result.stdout.trim()
const stderr = result.stderr.trim()
- if (result.code === 0 && stdout) {
+ if (result.code === RTK_EXIT_CODE_REWRITTEN && stdout) {
return {
status: 'rewritten',
command: stdout
}
}
- if (result.code === 3 && stdout && /No hook installed/i.test(stderr)) {
+ if (
+ result.code === RTK_EXIT_CODE_MISSING_HOOK &&
+ stdout &&
+ RTK_MISSING_HOOK_PATTERN.test(stderr)
+ ) {
return {
status: 'rewritten',
command: stdout
}
}
- if (result.code === 1) {
+ if (result.code === RTK_EXIT_CODE_BYPASS || result.code === RTK_EXIT_CODE_DENY) {
return {
status: 'bypass',
- message: 'RTK rewrite did not match this command'
+ message: result.code === RTK_EXIT_CODE_DENY
+ ? 'RTK deny rule matched'
+ : 'RTK rewrite did not match this command'
}
}The exit code 3 + "No hook installed" pattern also warrants a comment explaining why a normally-"ask" exit code is being treated as "rewritten" here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function classifyRtkRewriteResult(result: CommandResult): RtkRewriteResult { | |
| const stdout = result.stdout.trim() | |
| const stderr = result.stderr.trim() | |
| if (result.code === 0 && stdout) { | |
| return { | |
| status: 'rewritten', | |
| command: stdout | |
| } | |
| } | |
| if (result.code === 3 && stdout && /No hook installed/i.test(stderr)) { | |
| return { | |
| status: 'rewritten', | |
| command: stdout | |
| } | |
| } | |
| if (result.code === 1) { | |
| return { | |
| status: 'bypass', | |
| message: 'RTK rewrite did not match this command' | |
| } | |
| } | |
| return { | |
| status: 'failure', | |
| message: stderr || stdout || 'rtk rewrite failed' | |
| } | |
| } | |
| const RTK_EXIT_CODE_REWRITTEN = 0 | |
| const RTK_EXIT_CODE_BYPASS = 1 | |
| const RTK_EXIT_CODE_DENY = 2 | |
| const RTK_EXIT_CODE_MISSING_HOOK = 3 | |
| const RTK_MISSING_HOOK_PATTERN = /No hook installed/i | |
| function classifyRtkRewriteResult(result: CommandResult): RtkRewriteResult { | |
| const stdout = result.stdout.trim() | |
| const stderr = result.stderr.trim() | |
| if (result.code === RTK_EXIT_CODE_REWRITTEN && stdout) { | |
| return { | |
| status: 'rewritten', | |
| command: stdout | |
| } | |
| } | |
| if ( | |
| result.code === RTK_EXIT_CODE_MISSING_HOOK && | |
| stdout && | |
| RTK_MISSING_HOOK_PATTERN.test(stderr) | |
| ) { | |
| return { | |
| status: 'rewritten', | |
| command: stdout | |
| } | |
| } | |
| if (result.code === RTK_EXIT_CODE_BYPASS || result.code === RTK_EXIT_CODE_DENY) { | |
| return { | |
| status: 'bypass', | |
| message: result.code === RTK_EXIT_CODE_DENY | |
| ? 'RTK deny rule matched' | |
| : 'RTK rewrite did not match this command' | |
| } | |
| } | |
| return { | |
| status: 'failure', | |
| message: stderr || stdout || 'rtk rewrite failed' | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/lib/agentRuntime/rtkRuntimeService.ts` around lines 120 - 149,
Replace the magic numbers and inline regex in classifyRtkRewriteResult with
named SCREAMING_SNAKE_CASE constants (e.g. RTK_EXIT_SUCCESS, RTK_EXIT_BYPASS,
RTK_EXIT_DENY, RTK_EXIT_NO_HOOK and NO_HOOK_REGEX) and update the logic so that
exit code 2 (RTK_EXIT_DENY) returns the same bypass result as exit code 1; keep
the existing special-case that treats RTK_EXIT_NO_HOOK + "No hook installed" as
a rewritten result but add a short comment explaining why an otherwise-"ask"
exit code is treated as "rewritten". Ensure you update the checks in
classifyRtkRewriteResult to use these constants (and the NO_HOOK_REGEX) instead
of hardcoded numbers/strings.
Summary by CodeRabbit
Refactor
Tests