feat: custom tools — inject user-defined shell-command tools#648
Open
y0usaf wants to merge 9 commits intoPiebald-AI:mainfrom
Open
feat: custom tools — inject user-defined shell-command tools#648y0usaf wants to merge 9 commits intoPiebald-AI:mainfrom
y0usaf wants to merge 9 commits intoPiebald-AI:mainfrom
Conversation
On NixOS with Bun, node-lief fails to load because libstdc++.so.6 is not on the system library path. Previously this produced a generic "Could not extract JS from native binary" error with no hint about the cause or how to fix it. Now nativeInstallationLoader.ts tracks the load error, and installationDetection.ts surfaces it with a NixOS-specific suggestion to use Node.js instead (nix shell nixpkgs#nodejs -c npx tweakcc).
LIEF crashes (SIGSEGV) when parsing large ELF Bun binaries (~237MB). On Linux/NixOS, Bun stores its data as a plain file overlay — no ELF section parsing needed. This adds raw file-based extraction and repacking for ELF, skipping LIEF entirely for that format. LIEF is still used for Mach-O and PE where it reads actual binary sections that can't be found by tail-reading alone.
- isELFFile: use try/finally to prevent fd leak; validate bytesRead - extractBunDataFromELFRaw: correct docblock (uses BunOffsets.byteCount, not footer) - repackELFRaw: derive elfSize from BunOffsets.byteCount instead of the unreliable trailing u64 footer - nativeInstallationLoader: clear loadError on successful module load - installationDetection: tighten NixOS hint to only trigger on shared-object errors (drop the over-broad 'No such file' match) - config.test: add two tests covering enriched error message paths
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds first-class customTools (types, default setting, config validation), a patch that injects generated tool objects into Claude Code bundles (two injection strategies) with tests, and introduces ELF raw Bun extraction/repack plus native module load-error reporting. Changes
Sequence DiagramsequenceDiagram
participant User
participant Config
participant Patcher
participant Bundle
participant Executor
User->>Config: add settings.customTools
Config->>Patcher: provide validated customTools
Patcher->>Patcher: detect buildTool & getCwd names
Patcher->>Patcher: generate tool objects (validate/dedupe)
Patcher->>Bundle: inject generated tool objects (Strategy A/B)
User->>Bundle: trigger tool usage
Bundle->>Executor: call tool.call(params)
Executor->>Executor: validate inputs, template command, set cwd/env/timeout
Executor->>Executor: run spawnSync and collect result
Executor-->>Bundle: return execution result
Bundle-->>User: render tool result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 |
y0usaf
force-pushed
the
feat/custom-tools
branch
from
b07b3e0 to
944035a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/tests/config.test.ts (1)
1528-1610: Add a non-shared-object load-error case here too.Both new tests use the same
cannot open shared objectmessage, so they only cover the branch that appends the NixOS hint. Please add one generic load failure case to prove we still include the load reason without the Nix-specific note.As per coding guidelines,
**/*.test.{ts,tsx}: Test edge cases and error conditions in test files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/config.test.ts` around lines 1528 - 1610, Add a third test case similar to the two existing ones that stubs nativeInstallation.getNativeModuleLoadError to return a non-shared-object message (e.g. "unexpected header in node-lief") and otherwise sets up the same mocks (whichMock, fs.stat/realpath/open, lstatSpy, mockMagicInstance.detect, nativeInstallation.extractClaudeJsFromNativeInstallation) and then assert findClaudeCodeInstallation(mockConfig, { interactive: true }) rejects with an error that includes that exact load message (verifying findClaudeCodeInstallation and nativeInstallation.getNativeModuleLoadError handling without triggering the NixOS hint).
🤖 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/config.ts`:
- Around line 223-229: The current mapping of config.settings.customTools uses
deepMergeWithDefaults(tool, DEFAULT_CUSTOM_TOOL), which backfills required
execution fields (e.g., command) and turns malformed entries into executable
placeholders; update the logic in the config.settings.customTools handling to
first validate that each tool contains required properties (such as command and
any other required metadata) and either reject/drop the entry or throw a config
error, and only apply defaults for optional fields (merge only optional keys)
for valid tools; reference the mapping that calls deepMergeWithDefaults and the
DEFAULT_CUSTOM_TOOL constant and replace it with a two-step process: validate
required fields on each tool, then selectively merge optional defaults for the
validated tool.
In `@src/nativeInstallation.ts`:
- Around line 791-799: The current branch unconditionally uses
extractBunDataFromELFRaw for any ELF (isELFFile) which breaks section-based ELF
builds; update the logic in the nativeInstallationPath handling so you first
detect the legacy overlay footer (or attempt raw parsing and check its success)
before assigning ({ bunOffsets, bunData, moduleStructSize } =
extractBunDataFromELFRaw(nativeInstallationPath)); if the raw helper fails or
the footer is not present, fall back to the LIEF path that calls
LIEF.parse(nativeInstallationPath) and getBunData(binary) (and ensure
repackELFSection remains used for section-based ELFs) so only true
legacy-overlay ELFs use the raw helpers.
In `@src/patches/customTools.ts`:
- Around line 71-72: The code currently JSON-serializes tool.shell (shellJson)
but call() always uses the POSIX '-c' flag, which breaks non-POSIX shells (e.g.,
cmd.exe). Fix by either validating tool.shell in the parsing/creation path
(reject or normalize non-POSIX shells) or, preferably, add an explicit
shellExecFlag option (e.g., shellExecFlag or execFlag) to the tool config and
use that in call() instead of hardcoding '-c'; implement a default mapping for
common shells (bash/sh -> '-c', cmd.exe -> '/c', powershell -> '-Command') and
fall back to a validation/error if unknown, and update all call() usages
(including the locations referenced around lines 145-150) to read the exec flag
instead of assuming '-c'.
- Around line 92-97: Create a single, reusable escaped substitution template and
use it everywhere instead of duplicating logic: replace the current per-site
generation (the substitutions const, the inline replacement in
renderToolUseMessage, and the uses at the later sites) with one variable like
escapedSubstitution that (1) uses a safe regex construction (escape the
parameter name before embedding it into a RegExp or build the RegExp with new
RegExp(...)) to avoid invalid regex for names like "foo/bar", and (2) uses a
consistent fallback (pick one fallback, e.g. "?" or "" and apply it everywhere).
Add or reuse a small helper escapeRegExp(name: string): string if needed, then
reference escapeRegExp(k) when building the regex and use the same fallback
expression (String(args[KEY] ?? FALLBACK)) in substitutions,
renderToolUseMessage, and the two reuse sites so the displayed and executed
commands match.
---
Nitpick comments:
In `@src/tests/config.test.ts`:
- Around line 1528-1610: Add a third test case similar to the two existing ones
that stubs nativeInstallation.getNativeModuleLoadError to return a
non-shared-object message (e.g. "unexpected header in node-lief") and otherwise
sets up the same mocks (whichMock, fs.stat/realpath/open, lstatSpy,
mockMagicInstance.detect,
nativeInstallation.extractClaudeJsFromNativeInstallation) and then assert
findClaudeCodeInstallation(mockConfig, { interactive: true }) rejects with an
error that includes that exact load message (verifying
findClaudeCodeInstallation and nativeInstallation.getNativeModuleLoadError
handling without triggering the NixOS hint).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 266a94af-94be-48b9-9740-12268989e223
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
src/config.tssrc/defaultSettings.tssrc/installationDetection.tssrc/nativeInstallation.tssrc/nativeInstallationLoader.tssrc/patches/customTools.tssrc/patches/index.tssrc/tests/config.test.tssrc/types.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/patches/customTools.ts (2)
145-150:⚠️ Potential issue | 🟠 MajorThe
-cflag is POSIX-specific; non-POSIX shells will fail.The
shellfield is configurable (e.g., users could specifycmd.exeorpowershell), but the execution always passes["-c", cmd]. This breaks on Windows shells:cmd.exerequires/c, PowerShell requires-Command.Either validate
shellto POSIX-compatible shells only, or make the exec flag configurable (e.g., add ashellExecFlagfield).🛠️ Option 1: Validate shell to POSIX-only
Add validation in
src/config.tswhen merging custom tools:const ALLOWED_SHELLS = ['sh', 'bash', 'zsh', 'fish', 'dash', 'ash']; if (tool.shell && !ALLOWED_SHELLS.includes(tool.shell)) { console.warn(`Custom tool "${tool.name}": shell "${tool.shell}" is not supported. Using "sh".`); tool.shell = 'sh'; }🛠️ Option 2: Add shellExecFlag to CustomTool interface
Extend the interface in
src/types.ts:export interface CustomTool { // ... existing fields ... /** * Flag passed to shell before the command. Defaults to "-c" for POSIX shells. * Use "/c" for cmd.exe, "-Command" for PowerShell. */ shellExecFlag?: string; }Then use it in the generated code:
-const result=spawnSync(${shellJson},["-c",cmd],{ +const result=spawnSync(${shellJson},[${JSON.stringify(tool.shellExecFlag ?? '-c')},cmd],{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.ts` around lines 145 - 150, Add a configurable exec-flag instead of hardcoding ["-c", cmd]: extend the CustomTool type to include an optional shellExecFlag (e.g., in src/types.ts/CustomTool) and update the code that builds args for spawnSync (see spawnSync call using shellJson and cmd in src/patches/customTools.ts) to use tool.shellExecFlag when present, otherwise default to "-c" for POSIX shells; also add a small mapping/validation for common shells (cmd.exe -> "/c", powershell -> "-Command") and fall back to "-c" or warn if unknown so non-POSIX shells no longer break.
92-105:⚠️ Potential issue | 🟠 MajorParameter names must be escaped before embedding in regex patterns.
Parameter names are interpolated directly into regex literals without escaping. A parameter named
foo/barorparam.namewill produce invalid regex syntax (e.g.,/\{\{foo/bar\}\}/g) and throw at runtime.Additionally, the fallback values differ: line 95 uses
""for the executed command while line 104 uses"?"for the UI preview. This causes command drift where the user approves a different command than what executes.🛠️ Proposed fix: escape parameter names and unify fallbacks
+// Escape special regex characters in parameter names +const escapeRegex = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + // Build parameter substitution code: one replace per param const substitutions = Object.keys(tool.parameters) .map( k => - `cmd=cmd.replace(/\\{\\{${k}\\}\\}/g,String(args[${JSON.stringify(k)}]??""));` + `cmd=cmd.replace(/\\{\\{${escapeRegex(k)}\\}\\}/g,String(args[${JSON.stringify(k)}]??""));` ) .join(''); // renderToolUseMessage: show the tool name and interpolated command so the // user can see what's about to run before approving. const renderToolUseMessage = `(input)=>{ let cmd=${cmdJson}; ${Object.keys(tool.parameters) - .map(k => `try{cmd=cmd.replace(/\\{\\{${k}\\}\\}/g,String(input[${JSON.stringify(k)}]??"?"));}catch(_){}`) + .map(k => `try{cmd=cmd.replace(/\\{\\{${escapeRegex(k)}\\}\\}/g,String(input[${JSON.stringify(k)}]??""));}catch(_){}`) .join('')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.ts` around lines 92 - 105, The regex interpolation for parameter keys in the substitutions string and in renderToolUseMessage is unsafe: escape each parameter name before embedding into the /.../g regex (e.g., use an escapeRegex helper to backslash-escape regex metacharacters for keys from tool.parameters) and make the fallback value consistent between the executed command and the preview (use the same fallback—prefer ""—for both the substitutions (args) and renderToolUseMessage (input)); update references to substitutions generation and the renderToolUseMessage construction (they use cmdJson, args, input, and tool.parameters) so both use escaped keys and identical fallbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/patches/customTools.ts`:
- Around line 145-150: Add a configurable exec-flag instead of hardcoding ["-c",
cmd]: extend the CustomTool type to include an optional shellExecFlag (e.g., in
src/types.ts/CustomTool) and update the code that builds args for spawnSync (see
spawnSync call using shellJson and cmd in src/patches/customTools.ts) to use
tool.shellExecFlag when present, otherwise default to "-c" for POSIX shells;
also add a small mapping/validation for common shells (cmd.exe -> "/c",
powershell -> "-Command") and fall back to "-c" or warn if unknown so non-POSIX
shells no longer break.
- Around line 92-105: The regex interpolation for parameter keys in the
substitutions string and in renderToolUseMessage is unsafe: escape each
parameter name before embedding into the /.../g regex (e.g., use an escapeRegex
helper to backslash-escape regex metacharacters for keys from tool.parameters)
and make the fallback value consistent between the executed command and the
preview (use the same fallback—prefer ""—for both the substitutions (args) and
renderToolUseMessage (input)); update references to substitutions generation and
the renderToolUseMessage construction (they use cmdJson, args, input, and
tool.parameters) so both use escaped keys and identical fallbacks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 422acb21-ad54-4746-bd4b-8a2177b98917
📒 Files selected for processing (5)
src/config.tssrc/defaultSettings.tssrc/patches/customTools.tssrc/patches/index.tssrc/types.ts
✅ Files skipped from review due to trivial changes (2)
- src/config.ts
- src/patches/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/defaultSettings.ts
- src/types.ts
Uses CC's own buildTool() factory (detected via regex in the minified bundle) so custom tools are constructed the same way every native CC tool is. Rendering uses React.createElement with the detected Text/Box component variables rather than plain strings. Permission checking delegates to BashTool so custom tool commands match the user's existing Bash allow/deny rules. New helpers in helpers.ts: - getCwdFuncName: detects the minified getCwd() for correct session cwd - findBuildToolFunc: detects the minified buildTool() factory function Patch strategies (same as before): - Strategy A: post-toolsets injection (appends after toolset filter) - Strategy B: direct spread into the tool aggregation array
y0usaf
force-pushed
the
feat/custom-tools
branch
from
944035a to
05167f3
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/patches/customTools.ts (2)
194-199:⚠️ Potential issue | 🟠 MajorMake the exec flag follow the configured shell.
tool.shellis configurable, butspawnSync()always receives['-c', cmd]. That only works for POSIX shells;cmd.exeneeds/c, PowerShell needs-Command, etc. Either validatetool.shelldown to POSIX shells or carry an exec-flag mapping/config alongside it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.ts` around lines 194 - 199, The current spawnSync invocation in customTools.ts always uses ['-c', cmd], which breaks non-POSIX shells; update the code that builds the spawnSync args (around spawnSync, cmd, and tool.shell) to choose the correct exec flag based on tool.shell (e.g., map 'sh'/'bash' -> '-c', 'cmd.exe' -> '/c', 'pwsh'/'powershell' -> '-Command') or validate/restrict tool.shell to POSIX only; replace the hard-coded ['-c', cmd] with a computed args array (using the mapping or validation) so spawnSync(spawn, args, ...) passes the proper flag for the configured shell and preserve existing env, cwd, timeout, and encoding behavior.
118-130:⚠️ Potential issue | 🟠 MajorEscape placeholder keys and reuse one substitution path.
Execution/classifier/permissions substitute missing params with
"", but the approval UI uses"?", so the user can approve a different command from the one that actually runs. Also,kis embedded directly into a regex literal, so keys likea.bover-match andfoo/barwill generate invalid JS in the patched bundle. Build one escapednew RegExp(...)template and reuse it everywhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.ts` around lines 118 - 130, The substitution helpers (makeSubst, argsSubst, inputSubst, safeInputSubst) embed raw parameter keys into regex literals and use different missing-value defaults ("" vs "?"), causing over-matching/invalid JS and mismatched preview vs runtime; fix by building a single escaped RegExp-based substitution template (escape special regex chars in each key and construct new RegExp(escapedKey,'g')) and reuse that same substitution code path for args and input and for safeInputSubst, and change the safe/preview missing-param replacement to the same empty-string behavior used by execution/classifier/permissions so the rendered approval command matches the actual runtime command.
🧹 Nitpick comments (2)
src/patches/customTools.test.ts (1)
117-374: Add regressions for prompt overrides and non-identifier parameter names.The suite covers the happy path well, but two brittle codegen branches still have no direct coverage:
tool.promptbypassing auto-prompt generation, and parameter keys that need regex escaping (for examplea.borfoo/bar). One focused test for each would catch future breakage quickly.As per coding guidelines "Test edge cases and error conditions in test files".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.test.ts` around lines 117 - 374, Add two tests to the writeCustomTools suite to cover the missing branches: (1) a test that verifies when a CustomTool has an explicit prompt (tool.prompt) the generator uses it rather than auto-generating one—call writeCustomTools with a tool containing a prompt string and assert the generated result contains that prompt text (reference writeCustomTools and tool.prompt); (2) a test that verifies parameter keys with non-identifier characters (e.g. "a.b" and "foo/bar") are handled/escaped correctly—construct a CustomTool whose parameters include those keys, run writeCustomTools, and assert the output contains those exact keys or the correctly-escaped/quoted form (reference writeCustomTools and the parameter handling/regex-escaping logic). Ensure both tests stub console.warn/error like the existing tests and restore spies in finally blocks.src/patches/customTools.ts (1)
309-331: Tighten both injection regexes to the emitted minified shape.These patterns are much looser than the rest of the patch set. For bundle patches,
\s*-tolerant matching makes it easier to hit the wrongelse/aggregation site or to pass tests against prettified fixtures while missing the real minified target.Based on learnings "In patches under src/patches/, assume code works with minified, original Claude Code installations. Patch regexes must be strict and match the minified format exactly (ignore formatting differences like whitespace/newlines)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.ts` around lines 309 - 331, The current loose regexes toolsetsPattern and originalPattern allow whitespace and can match non-minified/prettified code; tighten them to the exact minified emission shape (no \s* gaps) so we only match sequences like "}else{VAR=CALL; }let" and "letVAR=CALL," in the minified bundle. Replace toolsetsPattern with a strict form that disallows whitespace between tokens (e.g. match "}" "else" "{" then capture the aggregation var and the assemble-call ending with ";" before "let") and replace originalPattern with a strict "let([$\w]+)=([$\w]+\([$\w]+,[$\w]+\.tools,[$\w]+\))," style regex; keep using toolAggVar, toolsArrayCode and the same insertAt/injection logic after updating the patterns.
🤖 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/patches/customTools.ts`:
- Around line 255-262: The loop currently only checks custom tool names against
BUILTIN_TOOL_NAMES; additionally detect duplicate names among the customTools
array itself (e.g., using a Set to track seen tool.name values) and treat
duplicates as an error: when a duplicate is found log a similar error message
(referencing customTools and the conflicting tool.name) and return null to abort
injection; update the check around the for (const tool of customTools) loop that
references BUILTIN_TOOL_NAMES and tool.name to include this duplicate-name guard
so no two user-defined tools with the same name are injected.
---
Duplicate comments:
In `@src/patches/customTools.ts`:
- Around line 194-199: The current spawnSync invocation in customTools.ts always
uses ['-c', cmd], which breaks non-POSIX shells; update the code that builds the
spawnSync args (around spawnSync, cmd, and tool.shell) to choose the correct
exec flag based on tool.shell (e.g., map 'sh'/'bash' -> '-c', 'cmd.exe' -> '/c',
'pwsh'/'powershell' -> '-Command') or validate/restrict tool.shell to POSIX
only; replace the hard-coded ['-c', cmd] with a computed args array (using the
mapping or validation) so spawnSync(spawn, args, ...) passes the proper flag for
the configured shell and preserve existing env, cwd, timeout, and encoding
behavior.
- Around line 118-130: The substitution helpers (makeSubst, argsSubst,
inputSubst, safeInputSubst) embed raw parameter keys into regex literals and use
different missing-value defaults ("" vs "?"), causing over-matching/invalid JS
and mismatched preview vs runtime; fix by building a single escaped RegExp-based
substitution template (escape special regex chars in each key and construct new
RegExp(escapedKey,'g')) and reuse that same substitution code path for args and
input and for safeInputSubst, and change the safe/preview missing-param
replacement to the same empty-string behavior used by
execution/classifier/permissions so the rendered approval command matches the
actual runtime command.
---
Nitpick comments:
In `@src/patches/customTools.test.ts`:
- Around line 117-374: Add two tests to the writeCustomTools suite to cover the
missing branches: (1) a test that verifies when a CustomTool has an explicit
prompt (tool.prompt) the generator uses it rather than auto-generating one—call
writeCustomTools with a tool containing a prompt string and assert the generated
result contains that prompt text (reference writeCustomTools and tool.prompt);
(2) a test that verifies parameter keys with non-identifier characters (e.g.
"a.b" and "foo/bar") are handled/escaped correctly—construct a CustomTool whose
parameters include those keys, run writeCustomTools, and assert the output
contains those exact keys or the correctly-escaped/quoted form (reference
writeCustomTools and the parameter handling/regex-escaping logic). Ensure both
tests stub console.warn/error like the existing tests and restore spies in
finally blocks.
In `@src/patches/customTools.ts`:
- Around line 309-331: The current loose regexes toolsetsPattern and
originalPattern allow whitespace and can match non-minified/prettified code;
tighten them to the exact minified emission shape (no \s* gaps) so we only match
sequences like "}else{VAR=CALL; }let" and "letVAR=CALL," in the minified bundle.
Replace toolsetsPattern with a strict form that disallows whitespace between
tokens (e.g. match "}" "else" "{" then capture the aggregation var and the
assemble-call ending with ";" before "let") and replace originalPattern with a
strict "let([$\w]+)=([$\w]+\([$\w]+,[$\w]+\.tools,[$\w]+\))," style regex; keep
using toolAggVar, toolsArrayCode and the same insertAt/injection logic after
updating the patterns.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6bcf5c7-6cc9-43c9-afb6-ed1ee13c8b55
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
src/defaultSettings.tssrc/installationDetection.tssrc/nativeInstallation.tssrc/nativeInstallationLoader.tssrc/patches/customTools.test.tssrc/patches/customTools.tssrc/patches/helpers.tssrc/patches/index.tssrc/tests/config.test.tssrc/types.ts
✅ Files skipped from review due to trivial changes (1)
- src/patches/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/defaultSettings.ts
- src/types.ts
- customTools.ts: escape param names in regex literals to handle special chars - customTools.ts: use consistent empty-string fallback in all substitution sites - customTools.ts: detect duplicate custom tool names (not just built-in collisions) - config.ts: validate required CustomTool fields when loading config; drop invalid entries - nativeInstallation.ts: gate raw ELF path on legacy footer detection; fall back to LIEF - config.test.ts: fix broken tests (use vi.spyOn for getNativeModuleLoadError) - config.test.ts: add test for generic (non-shared-object) load error without NixOS hint
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/patches/customTools.ts (2)
133-134:⚠️ Potential issue | 🟠 Major
tool.shellis broader than the hardcoded-cexecution path.
spawnSync(shell, ['-c', cmd])only works for POSIX-style shells.
Values likecmd.exe,powershell, orpwshneed different flags, so
those configs are accepted but won't execute correctly. Either validate
tool.shellto POSIX shells only, or pair it with an exec-flag mapping.♻️ Proposed fix
- const shellJson = JSON.stringify(tool.shell ?? 'sh'); + const shell = tool.shell ?? 'sh'; + const execFlag = + /^(cmd(?:\.exe)?)$/i.test(shell) + ? '/c' + : /^(powershell(?:\.exe)?|pwsh)$/i.test(shell) + ? '-Command' + : '-c'; + const shellJson = JSON.stringify(shell); ... - const result=spawnSync(${shellJson},["-c",cmd],{ + const result=spawnSync(${shellJson},[${JSON.stringify(execFlag)},cmd],{Also applies to: 244-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.ts` around lines 133 - 134, The code currently stringifies tool.shell into shellJson and then runs spawnSync(shell, ['-c', cmd]) which only works for POSIX shells; update the execution to derive the correct execution flag from the shell value (e.g., '-c' for sh/bash, '/c' for cmd.exe, '-Command' or '-c' for PowerShell variants) by adding a small helper like getShellExecFlag(shell) and use spawnSync(shell, [execFlag, cmd]); validate the returned execFlag (throw or log and skip if unknown) so configs like cmd.exe, powershell, and pwsh execute correctly; apply the same change where shellJson/timeoutVal and spawnSync are used elsewhere (the other block referenced around the later range).
156-179:⚠️ Potential issue | 🟠 MajorStop emitting regex literals for placeholder replacement.
escapeForRegex()still leaves/untouched, so a parameter like
foo/barproduces invalid generated code
(/\{\{foo/bar\}\}/g) and can break the patched bundle. Build the
matcher withnew RegExp(...)instead of a regex literal, or escape the
literal delimiter too.♻️ Proposed fix
const escapeForRegex = (s: string): string => s.replace(/[.*+?^${}()|[\]\\]/g, '\\\\$&'); + const makeRegex = (name: string): string => + `new RegExp(${JSON.stringify(`\\{\\{${escapeForRegex(name)}\\}\\}`)},'g')`; const makeSubst = (varName: string): string => Object.keys(tool.parameters) .map( k => - `cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(${varName}[${JSON.stringify(k)}]??""));` + `cmd=cmd.replace(${makeRegex(k)},String(${varName}[${JSON.stringify(k)}]??""));` ) .join(''); const safeInputSubst = Object.keys(tool.parameters) .map( k => - `try{cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(input[${JSON.stringify(k)}]??""));}catch(_){}` + `try{cmd=cmd.replace(${makeRegex(k)},String(input[${JSON.stringify(k)}]??""));}catch(_){}` ) .join('');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/customTools.ts` around lines 156 - 179, The generated replacement currently emits regex literals (see makeSubst, argsSubst, inputSubst, safeInputSubst and escapeForRegex) which fails for parameter names containing '/'; stop emitting regex literals and build matchers with the RegExp constructor instead. Update the template generation in makeSubst and the safeInputSubst mapping to produce code that calls new RegExp(...) with a properly escaped pattern string (escapeForRegex should either also escape '/' or you should escape it when interpolating into the string passed to RegExp) and the 'g' flag, and keep the same String(...??"") fallback logic; ensure the same change is applied where safeInputSubst currently emits try/catch per param.
🧹 Nitpick comments (1)
src/nativeInstallation.ts (1)
1520-1541: Consider extracting atomic file write logic to reduce duplication.The temp-file-then-rename pattern with busy-file error handling is duplicated from
atomicWriteBinary. A shared helper could simplify maintenance.♻️ Example extraction
function atomicWriteBuffer( buffer: Buffer, outputPath: string, originalPath: string ): void { const tempPath = outputPath + '.tmp'; fs.writeFileSync(tempPath, buffer); const origStat = fs.statSync(originalPath); fs.chmodSync(tempPath, origStat.mode); try { fs.renameSync(tempPath, outputPath); } catch (error) { try { fs.unlinkSync(tempPath); } catch { // ignore cleanup errors } if ( error instanceof Error && 'code' in error && (error.code === 'ETXTBSY' || error.code === 'EBUSY' || error.code === 'EPERM') ) { throw new Error( 'Cannot update the Claude executable while it is running.\n' + 'Please close all Claude instances and try again.' ); } throw error; } }Then
repackELFRawbecomes:- const tempPath = outputPath + '.tmp'; - fs.writeFileSync(tempPath, newBinary); - const origStat = fs.statSync(binPath); - fs.chmodSync(tempPath, origStat.mode); - try { - fs.renameSync(tempPath, outputPath); - } catch (error) { - try { - fs.unlinkSync(tempPath); - } catch { - // ignore cleanup errors - } - if ( - error instanceof Error && - 'code' in error && - (error.code === 'ETXTBSY' || - error.code === 'EBUSY' || - error.code === 'EPERM') - ) { - throw new Error( - 'Cannot update the Claude executable while it is running.\n' + - 'Please close all Claude instances and try again.' - ); - } - throw error; - } + atomicWriteBuffer(newBinary, outputPath, binPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nativeInstallation.ts` around lines 1520 - 1541, The temp-file-then-rename pattern and busy-file error handling in repackELFRaw is duplicated from atomicWriteBinary; extract this into a shared helper (e.g., atomicWriteBuffer or atomicReplaceFile) that takes the Buffer (or source path), outputPath and originalPath, performs write to tempPath, copies mode from original via fs.statSync+fs.chmodSync, attempts fs.renameSync and on failure unlinks tempPath and maps ETXTBSY/EBUSY/EPERM to the existing "Cannot update the Claude executable while it is running" Error; then replace the duplicated logic in repackELFRaw (and any other caller) to call the new helper and remove the duplicated try/catch block around renameSync.
🤖 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/config.ts`:
- Around line 230-245: The validation assumes config.settings.customTools is a
well-formed array of objects and uses JSON.stringify(tool) which can leak
secrets; instead first guard that config.settings.customTools is an Array (if
not, set to []), then iterate with index (e.g., replace the .filter call) and
for each entry validate types explicitly: ensure typeof tool === "object" &&
tool !== null, typeof tool.name === "string" && tool.name.trim() !== "", typeof
tool.command === "string", typeof tool.description === "string", and that
tool.parameters is the expected shape (use Array.isArray(tool.parameters) or
typeof tool.parameters === "object" depending on what writeCustomTools()
expects). For entries that fail, log only the index, the tool.name (if present),
and a list of the invalid/missing keys (do not JSON.stringify the whole tool),
and skip/drop those entries so writeCustomTools() receives only well-typed
tools.
In `@src/tests/config.test.ts`:
- Around line 1562-1568: The test currently only checks the generic error
prefix; update the assertion so it verifies the concrete load reason returned by
the mock (e.g. assert the error message contains the unique substring
"libstdc++.so.6") by changing the expectation on findClaudeCodeInstallation
(which is mocked via vi.spyOn(nativeInstallation, 'getNativeModuleLoadError')
and uses mockConfig) to check for that specific substring (use rejects.toThrow
with the full/partial string or rejects.toMatch if preferred) so the test fails
if the detailed reason is dropped.
---
Duplicate comments:
In `@src/patches/customTools.ts`:
- Around line 133-134: The code currently stringifies tool.shell into shellJson
and then runs spawnSync(shell, ['-c', cmd]) which only works for POSIX shells;
update the execution to derive the correct execution flag from the shell value
(e.g., '-c' for sh/bash, '/c' for cmd.exe, '-Command' or '-c' for PowerShell
variants) by adding a small helper like getShellExecFlag(shell) and use
spawnSync(shell, [execFlag, cmd]); validate the returned execFlag (throw or log
and skip if unknown) so configs like cmd.exe, powershell, and pwsh execute
correctly; apply the same change where shellJson/timeoutVal and spawnSync are
used elsewhere (the other block referenced around the later range).
- Around line 156-179: The generated replacement currently emits regex literals
(see makeSubst, argsSubst, inputSubst, safeInputSubst and escapeForRegex) which
fails for parameter names containing '/'; stop emitting regex literals and build
matchers with the RegExp constructor instead. Update the template generation in
makeSubst and the safeInputSubst mapping to produce code that calls new
RegExp(...) with a properly escaped pattern string (escapeForRegex should either
also escape '/' or you should escape it when interpolating into the string
passed to RegExp) and the 'g' flag, and keep the same String(...??"") fallback
logic; ensure the same change is applied where safeInputSubst currently emits
try/catch per param.
---
Nitpick comments:
In `@src/nativeInstallation.ts`:
- Around line 1520-1541: The temp-file-then-rename pattern and busy-file error
handling in repackELFRaw is duplicated from atomicWriteBinary; extract this into
a shared helper (e.g., atomicWriteBuffer or atomicReplaceFile) that takes the
Buffer (or source path), outputPath and originalPath, performs write to
tempPath, copies mode from original via fs.statSync+fs.chmodSync, attempts
fs.renameSync and on failure unlinks tempPath and maps ETXTBSY/EBUSY/EPERM to
the existing "Cannot update the Claude executable while it is running" Error;
then replace the duplicated logic in repackELFRaw (and any other caller) to call
the new helper and remove the duplicated try/catch block around renameSync.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ecfb591c-710c-4ab1-9359-8f6dbf77dc0d
📒 Files selected for processing (4)
src/config.tssrc/nativeInstallation.tssrc/patches/customTools.tssrc/tests/config.test.ts
Contributor
Author
|
Addressed all review comments:
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/patches/customTools.ts`:
- Around line 67-70: The generatePromptString function currently treats a falsy
prompt as missing, so a deliberate empty string prompt gets ignored; change the
check to explicitly test for undefined (e.g., if (tool.prompt !== undefined)) so
that an explicit '' is honored and returned; update the logic in
generatePromptString that references CustomTool.prompt to return tool.prompt
when it is defined rather than when it is truthy.
- Around line 218-236: The generated handlers (inputSchema.safeParse/parse,
toAutoClassifierInput, checkPermissions, and validateInput) assume input/args
are objects/arrays and can throw on null/undefined; ensure you normalize at the
top of those functions by coercing input to a safe default (e.g., input = input
?? {} and const args = Array.isArray(input.args) ? input.args : []) before any
indexing or substitutions, and update toAutoClassifierInput and checkPermissions
to use the normalized variables (refer to the functions/methods named
inputSchema:{safeParse,parse}, toAutoClassifierInput, checkPermissions, and
validateInput) so validateInput can return structured errors instead of runtime
TypeErrors.
- Around line 175-193: makeSubst and safeInputSubst currently pass a string to
String.prototype.replace which allows $-sequences in parameter values to be
interpreted; change the generated replacement to use a function callback so
values are inserted literally. Update makeSubst's generation from
`cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(${varName}[${JSON.stringify(k)}]??""));`
to use a replacer function like
`cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,()=>String(${varName}[${JSON.stringify(k)}]??""));`
and change each entry in safeInputSubst similarly from
`cmd=cmd.replace(...,String(input[...]))` to
`cmd=cmd.replace(...,()=>String(input[...]??""));` so argsSubst, inputSubst and
safeInputSubst all use function callbacks and avoid `$`-sequence interpretation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f51ed78f-4543-4bf3-94d2-79ccfe7c83a3
📒 Files selected for processing (3)
src/config.tssrc/patches/customTools.tssrc/tests/config.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/config.ts
- src/tests/config.test.ts
Comment on lines
+67
to
+70
| const generatePromptString = (tool: CustomTool): string => { | ||
| if (tool.prompt) { | ||
| return tool.prompt; | ||
| } |
Contributor
There was a problem hiding this comment.
Honor an explicit empty prompt override.
if (tool.prompt) treats prompt: '' as “no override” and regenerates the default prompt instead. Since CustomTool.prompt is typed as string | undefined, this should check for undefined.
Proposed fix
- if (tool.prompt) {
+ if (tool.prompt !== undefined) {
return tool.prompt;
}📝 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.
Suggested change
| const generatePromptString = (tool: CustomTool): string => { | |
| if (tool.prompt) { | |
| return tool.prompt; | |
| } | |
| const generatePromptString = (tool: CustomTool): string => { | |
| if (tool.prompt !== undefined) { | |
| return tool.prompt; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/patches/customTools.ts` around lines 67 - 70, The generatePromptString
function currently treats a falsy prompt as missing, so a deliberate empty
string prompt gets ignored; change the check to explicitly test for undefined
(e.g., if (tool.prompt !== undefined)) so that an explicit '' is honored and
returned; update the logic in generatePromptString that references
CustomTool.prompt to return tool.prompt when it is defined rather than when it
is truthy.
Comment on lines
+175
to
+193
| const makeSubst = (varName: string): string => | ||
| Object.keys(tool.parameters) | ||
| .map( | ||
| k => | ||
| `cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(${varName}[${JSON.stringify(k)}]??""));` | ||
| ) | ||
| .join(''); | ||
| const argsSubst = makeSubst('args'); | ||
| const inputSubst = makeSubst('input'); | ||
|
|
||
| // Safe substitution (try/catch per param) for renderToolUseMessage, which | ||
| // receives partial input while the model is still streaming parameters. | ||
| // Uses the same empty-string fallback as argsSubst/inputSubst for consistency. | ||
| const safeInputSubst = Object.keys(tool.parameters) | ||
| .map( | ||
| k => | ||
| `try{cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(input[${JSON.stringify(k)}]??""));}catch(_){}` | ||
| ) | ||
| .join(''); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that JS replacement strings rewrite $-sequences instead of inserting them literally.
node - <<'NODE'
const template = 'echo {{arg}}';
for (const value of ['$$', '$&', '$`', "$'"]) {
const out = template.replace(/\{\{arg\}\}/g, String(value));
console.log(JSON.stringify(value), '=>', JSON.stringify(out));
}
NODERepository: Piebald-AI/tweakcc
Length of output: 138
🏁 Script executed:
sed -n '175,193p' src/patches/customTools.tsRepository: Piebald-AI/tweakcc
Length of output: 834
🏁 Script executed:
rg 'replace\(' src/patches/customTools.ts -A 2 -B 2Repository: Piebald-AI/tweakcc
Length of output: 753
🏁 Script executed:
node - <<'NODE'
// Test both string replacement and callback replacement
const template = 'echo {{arg}}';
const testValues = ['$$', '$&', '$`', "$'", 'normal', 'test$value'];
console.log('=== String Replacement (VULNERABLE) ===');
for (const value of testValues) {
const out = template.replace(/\{\{arg\}\}/g, String(value));
console.log(JSON.stringify(value).padEnd(15), '=>', JSON.stringify(out));
}
console.log('\n=== Callback Replacement (PROPOSED FIX) ===');
for (const value of testValues) {
const out = template.replace(/\{\{arg\}\}/g, () => String(value));
console.log(JSON.stringify(value).padEnd(15), '=>', JSON.stringify(out));
}
NODERepository: Piebald-AI/tweakcc
Length of output: 523
🏁 Script executed:
# Check if custom tool parameters in this codebase might contain $ sequences
# Look for tool parameter examples or definitions
rg 'parameters.*:' src/patches/customTools.ts -A 5 -B 2 | head -40Repository: Piebald-AI/tweakcc
Length of output: 44
Use a replacer callback to prevent $-sequence interpretation in parameter values.
When String.prototype.replace() receives a string as the replacement argument, it interprets special sequences like $$, $&, $``, and $'rather than inserting the value literally. A parameter containing$, $&` → matched text, etc. Use an arrow function callback instead to preserve values exactly.
This affects both makeSubst() and safeInputSubst().
Proposed fix
const makeSubst = (varName: string): string =>
Object.keys(tool.parameters)
.map(
k =>
- `cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(${varName}[${JSON.stringify(k)}]??""));`
+ `cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,()=>String(${varName}[${JSON.stringify(k)}]??""));`
)
.join('');
@@
const safeInputSubst = Object.keys(tool.parameters)
.map(
k =>
- `try{cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(input[${JSON.stringify(k)}]??""));}catch(_){}`
+ `try{cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,()=>String(input[${JSON.stringify(k)}]??""));}catch(_){}`
)
.join('');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/patches/customTools.ts` around lines 175 - 193, makeSubst and
safeInputSubst currently pass a string to String.prototype.replace which allows
$-sequences in parameter values to be interpreted; change the generated
replacement to use a function callback so values are inserted literally. Update
makeSubst's generation from
`cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,String(${varName}[${JSON.stringify(k)}]??""));`
to use a replacer function like
`cmd=cmd.replace(/\\{\\{${escapeForRegex(k)}\\}\\}/g,()=>String(${varName}[${JSON.stringify(k)}]??""));`
and change each entry in safeInputSubst similarly from
`cmd=cmd.replace(...,String(input[...]))` to
`cmd=cmd.replace(...,()=>String(input[...]??""));` so argsSubst, inputSubst and
safeInputSubst all use function callbacks and avoid `$`-sequence interpretation.
Comment on lines
+218
to
+236
| inputSchema:{safeParse:(i)=>({success:true,data:i}),parse:(i)=>i}, | ||
| async description(){return ${descJson}}, | ||
| async prompt(){return ${promptJson}}, | ||
| isConcurrencySafe(){return false}, | ||
| isReadOnly(){return false}, | ||
| toAutoClassifierInput(input){ | ||
| let cmd=${cmdJson}; | ||
| ${inputSubst} | ||
| return cmd; | ||
| }, | ||
| checkPermissions(input,context){ | ||
| let cmd=${cmdJson}; | ||
| ${inputSubst} | ||
| const bashTool=context.options.tools.find(t=>t.name==="Bash"); | ||
| if(bashTool)return bashTool.checkPermissions({command:cmd,timeout:${timeoutVal}},context); | ||
| return Promise.resolve({behavior:"passthrough",message:"Permission required to run "+${nameJson}}); | ||
| }, | ||
| async validateInput(input){ | ||
| ${paramValidations} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the generated code dereferences input/args directly, and that nullish indexing throws.
sed -n '218,236p' src/patches/customTools.ts
sed -n '256,268p' src/patches/customTools.ts
node - <<'NODE'
for (const value of [undefined, null]) {
try {
console.log(value['toolArg']);
} catch (err) {
console.log(err.name + ': ' + err.message);
}
}
NODERepository: Piebald-AI/tweakcc
Length of output: 1403
Normalize input/args before indexing into them.
inputSchema.parse/safeParse accept anything, but the generated code immediately reads input[...] / args[...]. null or undefined payloads will throw TypeError before validateInput() can return a structured error.
Proposed fix
inputSchema:{safeParse:(i)=>({success:true,data:i}),parse:(i)=>i},
async description(){return ${descJson}},
async prompt(){return ${promptJson}},
isConcurrencySafe(){return false},
isReadOnly(){return false},
toAutoClassifierInput(input){
+ input=typeof input==="object"&&input!==null?input:{};
let cmd=${cmdJson};
${inputSubst}
return cmd;
},
checkPermissions(input,context){
+ input=typeof input==="object"&&input!==null?input:{};
let cmd=${cmdJson};
${inputSubst}
const bashTool=context.options.tools.find(t=>t.name==="Bash");
if(bashTool)return bashTool.checkPermissions({command:cmd,timeout:${timeoutVal}},context);
return Promise.resolve({behavior:"passthrough",message:"Permission required to run "+${nameJson}});
},
async validateInput(input){
+ if(typeof input!=="object"||input===null||Array.isArray(input))
+ return{result:false,message:"input must be an object",errorCode:1};
${paramValidations}
return{result:true};
},
@@
async call(args){
+ args=typeof args==="object"&&args!==null?args:{};
let cmd=${cmdJson};
${argsSubst}
const {spawnSync}=${requireFunc}("child_process");Applies to lines 218-236 and 256-268.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/patches/customTools.ts` around lines 218 - 236, The generated handlers
(inputSchema.safeParse/parse, toAutoClassifierInput, checkPermissions, and
validateInput) assume input/args are objects/arrays and can throw on
null/undefined; ensure you normalize at the top of those functions by coercing
input to a safe default (e.g., input = input ?? {} and const args =
Array.isArray(input.args) ? input.args : []) before any indexing or
substitutions, and update toAutoClassifierInput and checkPermissions to use the
normalized variables (refer to the functions/methods named
inputSchema:{safeParse,parse}, toAutoClassifierInput, checkPermissions, and
validateInput) so validateInput can return structured errors instead of runtime
TypeErrors.
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
1 participant
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
New helpers
Config shape
{ "settings": { "customTools": [ { "name": "DateTool", "description": "Get the current date and time", "parameters": {}, "command": "date" }, { "name": "WeatherTool", "description": "Get weather for a city", "parameters": { "city": { "type": "string", "description": "City name" } }, "command": "curl -s wttr.in/{{city}}?format=3", "shell": "bash", "timeout": 10000 } ] } }Optional fields:
shell(default"sh"),timeout(default30000ms),workingDir,env(extra env vars),prompt(override the auto-generated LLM description).Test plan
customTools— patch skips cleanly, no change to cli.js{{param}}placeholders substituted in rendered command and executionpromptoverride — custom prompt string is sent to the modelSummary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests