-
Notifications
You must be signed in to change notification settings - Fork 614
feat: add shell to powerpack #1178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add shell to powerpack #1178
Conversation
WalkthroughAdds cross-platform shell command execution to PowerpackServer with OS detection, shell selection (PowerShell on Windows, Bash on macOS/Linux), input validation, working-directory management, command execution and output formatting; also prevents terminal rendering for Changes
Sequence DiagramsequenceDiagram
actor User
participant Client
participant PowerpackServer
participant OSDetector
participant ShellExecutor
participant FileSystem
Client->>PowerpackServer: invoke run_shell_command(args)
PowerpackServer->>PowerpackServer: validate args (RunShellCommandArgsSchema)
alt first-time or shell not configured
PowerpackServer->>OSDetector: detect host OS
OSDetector-->>PowerpackServer: OS type
PowerpackServer->>PowerpackServer: select shell (PowerShell/Bash)
PowerpackServer->>FileSystem: ensureShellWorkdir()
FileSystem-->>PowerpackServer: workdir ready
end
PowerpackServer->>ShellExecutor: executeShellCommand(command, shell, workdir)
ShellExecutor->>ShellExecutor: construct shell args & spawn process
ShellExecutor-->>PowerpackServer: stdout + stderr or error
alt success
PowerpackServer-->>Client: formatted result (stdout/stderr)
else error
PowerpackServer-->>Client: error details and cleanup status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts (1)
108-123:ensureShellWorkdir()can throw before the try/catch; also temp dirs are never cleaned up.
app.getPath('temp')is outside the try block, so “app not ready” (or similar) will crash the constructor.fs.mkdtempSync(...)creates a unique directory, but nothing removes it later → temp-dir leak.private ensureShellWorkdir(): string { - const baseTempDir = app.getPath('temp') - const shellDirPrefix = path.join(baseTempDir, 'powerpack_shell_') - try { + const baseTempDir = app.getPath('temp') + const shellDirPrefix = path.join(baseTempDir, 'powerpack_shell_') return fs.mkdtempSync(shellDirPrefix) } catch (error) { - console.error('Failed to ensure shell workdir, falling back to temp path:', error) - return baseTempDir + console.error('Failed to ensure shell workdir, falling back to temp path:', error) + // Best-effort fallback (avoid throwing from fallback path as well) + try { + return app.getPath('temp') + } catch { + return process.cwd() + } } }Cleanup suggestion (pick one):
- Add a
dispose()onPowerpackServertofs.rmSync(this.shellWorkdir, { recursive: true, force: true })when it’s a mkdtemp-created dir, and ensure the owner calls it; or- Create a per-command mkdtemp dir and delete it in a
finally.
As per coding guidelines, also prefer structured logging overconsole.*.Also applies to: 287-297
🧹 Nitpick comments (2)
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts (2)
50-68: TightenRunShellCommandArgsSchemato reduce foot-guns (timeout bounds, command length, and workdir constraints).
Right now any-length commands + any timeout + arbitraryworkdirare accepted; even if intended, adding basic bounds will prevent accidental hangs and huge prompts.const RunShellCommandArgsSchema = z.object({ command: z .string() .min(1) + .max(4000) .describe( 'Shell command to execute. Provide the full command string including arguments and flags.' ), timeout: z .number() .optional() .default(60000) + .min(1000) + .max(5 * 60 * 1000) .describe('Command execution timeout in milliseconds, defaults to 60 seconds.'), workdir: z .string() .optional() .describe( 'Optional working directory. If omitted, uses an isolated temporary directory under the application temp path.' ) })
205-211: Error handling/logging: prefer structured logging + avoid dumping entire ZodError objects into messages.
${parsed.error}can be very noisy; considerparsed.error.issues(or.message) to keep responses concise.- New/changed
console.*logging should ideally belogger.*with stack/context, per coding guidelines.Also applies to: 440-441, 463-464, 500-501, 524-525, 553-554, 572-578
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts(16 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and maintain strict TypeScript type checking for all files
**/*.{ts,tsx}: Always use try-catch to handle possible errors in TypeScript code
Provide meaningful error messages when catching errors
Log detailed error logs including error details, context, and stack traces
Distinguish and handle different error types (UserError, NetworkError, SystemError, BusinessError) with appropriate handlers in TypeScript
Use structured logging with logger.error(), logger.warn(), logger.info(), logger.debug() methods from logging utilities
Do not suppress errors (avoid empty catch blocks or silently ignoring errors)
Provide user-friendly error messages for user-facing errors in TypeScript components
Implement error retry mechanisms for transient failures in TypeScript
Avoid logging sensitive information (passwords, tokens, PII) in logs
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Organize core business logic into dedicated Presenter classes, with one presenter per functional domain
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use EventBus from
src/main/eventbus.tsfor main-to-renderer communication, broadcasting events viamainWindow.webContents.send()
src/main/**/*.ts: Use EventBus pattern for inter-process communication within the main process to decouple modules
Use Electron's built-in APIs for file system and native dialogs instead of Node.js or custom implementations
src/main/**/*.ts: Electron main process code belongs insrc/main/with presenters inpresenter/(Window/Tab/Thread/Mcp/Config/LLMProvider) andeventbus.tsfor app events
Use the Presenter pattern in the main process for UI coordination
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/main/presenter/mcpPresenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Register new MCP tools in
mcpPresenter/index.tsafter implementing them ininMemoryServers/
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Do not include AI co-authoring information (e.g., 'Co-Authored-By: Claude') in git commits
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
**/*.{js,ts,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Write logs and comments in English
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
{src/main/presenter/**/*.ts,src/renderer/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Implement proper inter-process communication (IPC) patterns using Electron's ipcRenderer and ipcMain APIs
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/main/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Main process code for Electron should be placed in
src/main
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/**/*.{ts,tsx,vue,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with single quotes, no semicolons, and 100 character width
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use OxLint for linting JavaScript and TypeScript files
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Use camelCase for variable and function names in TypeScript files
Use PascalCase for type and class names in TypeScript
Use SCREAMING_SNAKE_CASE for constant names
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use EventBus for inter-process communication events
Files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
🧠 Learnings (7)
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/presenter/mcpPresenter/**/*.ts : Register new MCP tools in `mcpPresenter/index.ts` after implementing them in `inMemoryServers/`
Applied to files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
📚 Learning: 2025-11-25T05:27:12.209Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-11-25T05:27:12.209Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : In Provider implementations, handle native tool support by converting MCP tools to Provider format using `convertToProviderTools` and including them in the API request; for Providers without native function call support, prepare messages using `prepareFunctionCallPrompt` before making the API call
Applied to files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Convert MCP tools to provider-specific formats and normalize streaming responses to standard events in each provider implementation
Applied to files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
📚 Learning: 2025-11-25T05:26:35.317Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/error-logging.mdc:0-0
Timestamp: 2025-11-25T05:26:35.317Z
Learning: Applies to **/*.{ts,tsx} : Provide user-friendly error messages for user-facing errors in TypeScript components
Applied to files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
📚 Learning: 2025-11-25T05:26:35.317Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/error-logging.mdc:0-0
Timestamp: 2025-11-25T05:26:35.317Z
Learning: Applies to **/*.{ts,tsx} : Distinguish and handle different error types (UserError, NetworkError, SystemError, BusinessError) with appropriate handlers in TypeScript
Applied to files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
📚 Learning: 2025-11-25T05:26:35.317Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/error-logging.mdc:0-0
Timestamp: 2025-11-25T05:26:35.317Z
Learning: Applies to **/*.{ts,tsx} : Provide meaningful error messages when catching errors
Applied to files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
📚 Learning: 2025-11-25T05:27:39.200Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/provider-guidelines.mdc:0-0
Timestamp: 2025-11-25T05:27:39.200Z
Learning: Applies to **/*Provider**/index.ts : Error handling: use `error` event uniformly to carry error messages, avoid mixing errors into other event fields
Applied to files:
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
| type ShellEnvironment = { | ||
| platform: 'windows' | 'mac' | 'linux' | ||
| shellName: 'powershell' | 'bash' | ||
| shellExecutable: string | ||
| buildArgs: (command: string) => string[] | ||
| promptHint: string | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ShellEnvironment shape is sensible, but consider making promptHint deterministic (no trailing spaces) and future-proofing for more shells.
Minor nit: the Windows promptHint already includes a trailing space and is concatenated with another ' ' later, producing double-spacing.
🤖 Prompt for AI Agents
In src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts around
lines 16 to 22, remove trailing spaces from the promptHint values so they are
deterministic (no built-in space) and ensure any caller concatenates the
separating space when building prompts to avoid double-spacing; also make the
shellName type future-proof by widening the union to allow additional shells
(e.g., change "shellName: 'powershell' | 'bash'" to "shellName: 'powershell' |
'bash' | string" or simply "string") so new shell types can be added without
type changes.
| private detectShellEnvironment(): ShellEnvironment { | ||
| if (process.platform === 'win32') { | ||
| return { | ||
| platform: 'windows', | ||
| shellName: 'powershell', | ||
| shellExecutable: 'powershell.exe', | ||
| buildArgs: (command) => ['-Command', command], | ||
| promptHint: | ||
| 'Windows environment detected. Commands run with PowerShell, you can use built-in cmdlets and scripts. ' | ||
| } | ||
| } | ||
|
|
||
| const isMac = process.platform === 'darwin' | ||
|
|
||
| return { | ||
| platform: isMac ? 'mac' : 'linux', | ||
| shellName: 'bash', | ||
| shellExecutable: '/bin/bash', | ||
| buildArgs: (command) => ['-lc', command], | ||
| promptHint: isMac | ||
| ? 'macOS environment detected. Commands run with bash; macOS utilities like osascript, open, defaults are available.' | ||
| : 'Linux environment detected. Commands run with bash and typical GNU utilities are available.' | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the file structure and see how detectShellEnvironment is used
cd src/main/presenter/mcpPresenter/inMemoryServers && cat -n powerpackServer.ts | head -200Repository: ThinkInAIXYZ/deepchat
Length of output: 7150
🏁 Script executed:
# Search for other shell execution patterns in the codebase
rg "shellExecutable|buildArgs" -A 3 -B 3 src/main/Repository: ThinkInAIXYZ/deepchat
Length of output: 3439
🏁 Script executed:
# Check if there's any fallback logic for shell detection elsewhere
rg "/bin/bash|/bin/sh|which bash" src/Repository: ThinkInAIXYZ/deepchat
Length of output: 603
🏁 Script executed:
# Look for how these buildArgs are actually used
rg "buildArgs\(" src/main/Repository: ThinkInAIXYZ/deepchat
Length of output: 214
🏁 Script executed:
# Check the runShellCommand method implementation to see error handling
rg -A 20 "private.*runShellCommand|async.*runShellCommand" src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Check if there's any existing validation/fallback for shell executable
rg -B 5 -A 10 "execFile.*shellExecutable" src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 529
🏁 Script executed:
# Look at how other shell helpers handle missing bash
cat -n src/main/presenter/llmProviderPresenter/agent/shellEnvHelper.ts | head -100Repository: ThinkInAIXYZ/deepchat
Length of output: 3888
🏁 Script executed:
# Check what types of commands are executed through runShellCommand
rg -B 5 "runShellCommand" src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts | head -50Repository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Search for where runShellCommand is called to understand the context
rg "runShellCommand\(" src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts -A 3 -B 3Repository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Check if there are any explicit notes about why -lc is used
rg "-lc|login" src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts -B 5 -A 5Repository: ThinkInAIXYZ/deepchat
Length of output: 469
🏁 Script executed:
# Check what tools/commands are defined in powerpackServer
grep -n "tool\|command" src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts | head -40Repository: ThinkInAIXYZ/deepchat
Length of output: 1519
🏁 Script executed:
# Get a better picture of the tool definitions
sed -n '300,500p' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 7825
Add security flags and fallback for shell executables.
For non-interactive command execution, avoid loading user profiles: use -NoProfile with PowerShell, -NonInteractive for consistency, and -c instead of -lc for bash (login shells load user profiles, causing non-deterministic behavior and potential security issues). Also add a fallback to process.env.SHELL before defaulting to /bin/bash, matching the pattern already used elsewhere in the codebase.
private detectShellEnvironment(): ShellEnvironment {
if (process.platform === 'win32') {
return {
platform: 'windows',
shellName: 'powershell',
shellExecutable: 'powershell.exe',
- buildArgs: (command) => ['-Command', command],
+ buildArgs: (command) => ['-NoProfile', '-NonInteractive', '-Command', command],
promptHint:
'Windows environment detected. Commands run with PowerShell, you can use built-in cmdlets and scripts. '
}
}
const isMac = process.platform === 'darwin'
return {
platform: isMac ? 'mac' : 'linux',
shellName: 'bash',
- shellExecutable: '/bin/bash',
+ shellExecutable: process.env.SHELL || '/bin/bash',
- buildArgs: (command) => ['-lc', command],
+ buildArgs: (command) => ['-c', command],
promptHint: isMac
? 'macOS environment detected. Commands run with bash; macOS utilities like osascript, open, defaults are available.'
: 'Linux environment detected. Commands run with bash and typical GNU utilities are available.'
}
}📝 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.
| private detectShellEnvironment(): ShellEnvironment { | |
| if (process.platform === 'win32') { | |
| return { | |
| platform: 'windows', | |
| shellName: 'powershell', | |
| shellExecutable: 'powershell.exe', | |
| buildArgs: (command) => ['-Command', command], | |
| promptHint: | |
| 'Windows environment detected. Commands run with PowerShell, you can use built-in cmdlets and scripts. ' | |
| } | |
| } | |
| const isMac = process.platform === 'darwin' | |
| return { | |
| platform: isMac ? 'mac' : 'linux', | |
| shellName: 'bash', | |
| shellExecutable: '/bin/bash', | |
| buildArgs: (command) => ['-lc', command], | |
| promptHint: isMac | |
| ? 'macOS environment detected. Commands run with bash; macOS utilities like osascript, open, defaults are available.' | |
| : 'Linux environment detected. Commands run with bash and typical GNU utilities are available.' | |
| } | |
| } | |
| private detectShellEnvironment(): ShellEnvironment { | |
| if (process.platform === 'win32') { | |
| return { | |
| platform: 'windows', | |
| shellName: 'powershell', | |
| shellExecutable: 'powershell.exe', | |
| buildArgs: (command) => ['-NoProfile', '-NonInteractive', '-Command', command], | |
| promptHint: | |
| 'Windows environment detected. Commands run with PowerShell, you can use built-in cmdlets and scripts. ' | |
| } | |
| } | |
| const isMac = process.platform === 'darwin' | |
| return { | |
| platform: isMac ? 'mac' : 'linux', | |
| shellName: 'bash', | |
| shellExecutable: process.env.SHELL || '/bin/bash', | |
| buildArgs: (command) => ['-c', command], | |
| promptHint: isMac | |
| ? 'macOS environment detected. Commands run with bash; macOS utilities like osascript, open, defaults are available.' | |
| : 'Linux environment detected. Commands run with bash and typical GNU utilities are available.' | |
| } | |
| } |
| private async executeShellCommand( | ||
| command: string, | ||
| timeout: number, | ||
| workdir?: string | ||
| ): Promise<string> { | ||
| const { shellExecutable, buildArgs } = this.shellEnvironment | ||
| const execPromise = promisify(execFile)(shellExecutable, buildArgs(command), { | ||
| timeout, | ||
| cwd: workdir || this.shellWorkdir, | ||
| windowsHide: true | ||
| }) | ||
|
|
||
| const { stdout, stderr } = await execPromise | ||
| const outputParts: string[] = [] | ||
|
|
||
| if (stdout) { | ||
| outputParts.push(stdout.trim()) | ||
| } | ||
|
|
||
| if (stderr) { | ||
| outputParts.push(`STDERR:\n${stderr.trim()}`) | ||
| } | ||
|
|
||
| return outputParts.join('\n\n') || 'Command executed with no output' | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
executeShellCommand() should preserve stdout/stderr on non-zero exit + set maxBuffer.
Today, command failures/timeouts throw and the outer catch only returns error.message, often losing stderr (the most useful part). Also, default maxBuffer can error on moderately large output.
private async executeShellCommand(
command: string,
timeout: number,
workdir?: string
): Promise<string> {
const { shellExecutable, buildArgs } = this.shellEnvironment
- const execPromise = promisify(execFile)(shellExecutable, buildArgs(command), {
- timeout,
- cwd: workdir || this.shellWorkdir,
- windowsHide: true
- })
-
- const { stdout, stderr } = await execPromise
- const outputParts: string[] = []
+ try {
+ const { stdout, stderr } = await promisify(execFile)(shellExecutable, buildArgs(command), {
+ timeout,
+ cwd: workdir || this.shellWorkdir,
+ windowsHide: true,
+ maxBuffer: 10 * 1024 * 1024
+ })
+
+ const outputParts: string[] = []
- if (stdout) {
- outputParts.push(stdout.trim())
- }
+ if (stdout) outputParts.push(stdout.trim())
+ if (stderr) outputParts.push(`STDERR:\n${stderr.trim()}`)
- if (stderr) {
- outputParts.push(`STDERR:\n${stderr.trim()}`)
- }
+ return outputParts.join('\n\n') || 'Command executed with no output'
+ } catch (error) {
+ const err = error as any
+ const stdout = typeof err?.stdout === 'string' ? err.stdout : ''
+ const stderr = typeof err?.stderr === 'string' ? err.stderr : ''
+ const outputParts: string[] = []
+ if (stdout) outputParts.push(stdout.trim())
+ if (stderr) outputParts.push(`STDERR:\n${stderr.trim()}`)
+ const detail = outputParts.join('\n\n')
+ const baseMessage = error instanceof Error ? error.message : String(error)
+ throw new Error(detail ? `${baseMessage}\n\n${detail}` : baseMessage)
+ }
-
- return outputParts.join('\n\n') || 'Command executed with no output'
}🤖 Prompt for AI Agents
In src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts around
lines 261 to 285, update executeShellCommand so it sets a larger maxBuffer in
the execFile options (e.g. 10*1024*1024) and preserves stdout/stderr when the
child process exits non‑zero or times out: await the exec call inside a
try/catch, and on error extract and include error.stdout and error.stderr
(falling back to the error.message) in the returned/raised output instead of
dropping stderr; ensure cwd, timeout and windowsHide remain passed through and
return a combined string that contains trimmed stdout and stderr (with clear
labels) whether the command succeeded or failed.
| const shellDescription = | ||
| `${this.shellEnvironment.promptHint} ` + | ||
| 'Use this tool for day-to-day automation, file inspection, networking, and scripting. ' + | ||
| 'Provide a full shell command string; output includes stdout and stderr. ' | ||
|
|
||
| tools.push({ | ||
| name: 'run_shell_command', | ||
| description: shellDescription, | ||
| inputSchema: zodToJsonSchema(RunShellCommandArgsSchema) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for run_shell_command references
echo "=== Searching for run_shell_command ==="
rg -n --type=ts -C3 '\brun_shell_command\b' src/main/presenter/mcpPresenter/ || echo "No matches found"
echo ""
echo "=== Searching for PowerpackServer ==="
rg -n --type=ts -C2 '\bPowerpackServer\b' src/main/presenter/mcpPresenter/ || echo "No matches found"
echo ""
echo "=== Looking for index.ts in mcpPresenter ==="
fd -a '^index\.ts$' src/main/presenter/mcpPresenter || echo "Not found"
echo ""
echo "=== Checking PowerpackServer file structure ==="
wc -l src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts || echo "File not found"Repository: ThinkInAIXYZ/deepchat
Length of output: 3304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check PowerpackServer constructor and instantiation
echo "=== PowerpackServer constructor ==="
sed -n '1,50p' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts | head -50
echo ""
echo "=== Check lines around 384-393 and 496-514 ==="
sed -n '380,420p' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
echo ""
sed -n '490,520p' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 4674
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if MCP tools are registered elsewhere
echo "=== Check mcpPresenter index.ts for tool registration ==="
cat src/main/presenter/mcpPresenter/index.ts 2>/dev/null || echo "File not found"
echo ""
echo "=== Search for PowerpackServer instantiation ==="
rg -n --type=ts 'new PowerpackServer\|PowerpackServer\(' src/main/ || echo "No instantiation found"
echo ""
echo "=== Check for env variable patterns in PowerpackServer ==="
rg -n --type=ts 'ENABLE|env\?\.|\benv\b' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts | head -20Repository: ThinkInAIXYZ/deepchat
Length of output: 43930
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how PowerpackServer exposes tools
echo "=== PowerpackServer getTools or similar method ==="
grep -n "getTools\|listTools\|tools.push\|export" src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts | head -30
echo ""
echo "=== Full PowerpackServer class structure (first 150 lines after class definition) ==="
sed -n '103,250p' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts | head -80
echo ""
echo "=== Check builder.ts for how tools are used ==="
cat src/main/presenter/mcpPresenter/inMemoryServers/builder.ts
echo ""
echo "=== Check if index.ts references PowerpackServer tools ==="
rg -n "powerpack\|PowerpackServer" src/main/presenter/mcpPresenter/index.ts || echo "No references found"Repository: ThinkInAIXYZ/deepchat
Length of output: 5911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the method that handles tool listing for PowerpackServer
echo "=== Looking for handleListTools or ListToolsRequest handler ==="
sed -n '200,350p' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
echo ""
echo "=== Check setupRequestHandlers method ==="
sed -n '160,210p' src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 6094
Gate run_shell_command tool behind an explicit enable flag; add runtime guard in handler.
This tool enables arbitrary local command execution via the MCP interface. Add an enableShellCommandTool flag gated by environment variable (following the existing USE_E2B pattern), conditionally push the tool at lines 389–393, and add a runtime guard in the run_shell_command case handler at line 496:
export class PowerpackServer {
private server: Server
private useE2B: boolean = false
private e2bApiKey: string = ''
+ private readonly enableShellCommandTool: boolean
constructor(env?: Record<string, any>) {
this.parseE2BConfig(env)
+ this.enableShellCommandTool =
+ env?.ENABLE_SHELL_COMMAND_TOOL === true || env?.ENABLE_SHELL_COMMAND_TOOL === 'true'
this.runtimeHelper.initializeRuntimes()
// ...
}Then conditionally register the tool:
const shellDescription =
`${this.shellEnvironment.promptHint} ` +
'Use this tool for day-to-day automation, file inspection, networking, and scripting. ' +
'Provide a full shell command string; output includes stdout and stderr. '
- tools.push({
- name: 'run_shell_command',
- description: shellDescription,
- inputSchema: zodToJsonSchema(RunShellCommandArgsSchema)
- })
+ if (this.enableShellCommandTool) {
+ tools.push({
+ name: 'run_shell_command',
+ description: shellDescription,
+ inputSchema: zodToJsonSchema(RunShellCommandArgsSchema)
+ })
+ }And add a guard in the handler at line 496:
case 'run_shell_command': {
+ if (!this.enableShellCommandTool) {
+ throw new Error('Shell command execution is disabled by configuration')
+ }
const parsed = RunShellCommandArgsSchema.safeParse(args)Also applies to lines 496–514.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts around
lines 384–393 (and the run_shell_command handler around lines 496–514), gate the
run_shell_command tool behind a new boolean flag enableShellCommandTool derived
from an environment variable (following the existing USE_E2B pattern), only push
the tool into tools when that flag is true, and add a runtime guard at the start
of the run_shell_command case handler that checks the same flag and
throws/returns a clear error if the tool is disabled; follow the file's existing
env-config pattern and error handling style for naming, conditional
registration, and the runtime check so the tool is both not advertised when
disabled and cannot be executed if invoked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/renderer/src/components/message/MessageBlockToolCall.vue (1)
178-181: Use strict equality operator (===) instead of loose equality (==).Line 179 uses
==for comparison. While it works correctly here, prefer===for consistency and to avoid potential type coercion issues.Apply this diff:
- if (name == 'run_shell_command' && serverName === 'powerpack') { + if (name === 'run_shell_command' && serverName === 'powerpack') {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/message/MessageBlockToolCall.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.vue: Use Vue 3 Composition API for all components instead of Options API
Use Tailwind CSS with scoped styles for component styling
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/**/*.vue: All user-facing strings must use i18n keys via vue-i18n for internationalization
Ensure proper error handling and loading states in all UI components
Implement responsive design using Tailwind CSS utilities for all UI components
src/renderer/**/*.vue: Use composition API and declarative programming patterns; avoid options API
Structure files: exported component, composables, helpers, static content, types
Use PascalCase for component names (e.g., AuthWizard.vue)
Use Vue 3 with TypeScript, leveraging defineComponent and PropType
Use template syntax for declarative rendering
Use Shadcn Vue, Radix Vue, and Tailwind for components and styling
Implement responsive design with Tailwind CSS; use a mobile-first approach
Use Suspense for asynchronous components
Use <script setup> syntax for concise component definitions
Prefer 'lucide:' icon family as the primary choice for Iconify icons
Import Icon component from '@iconify/vue' and use with lucide icons following pattern '{collection}:{icon-name}'
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*.{vue,ts,tsx}: All user-facing strings must use i18n keys with vue-i18n framework in the renderer
Import and use useI18n() composable with the t() function to access translations in Vue components and TypeScript files
Use the dynamic locale.value property to switch languages at runtime
Avoid hardcoding user-facing text and ensure all user-visible text uses the i18n translation system
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/**/*.{vue,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Renderer process code should be placed in
src/renderer(Vue 3 application)
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability in Vue.js applications
Implement proper state management with Pinia in Vue.js applications
Utilize Vue Router for navigation and route management in Vue.js applications
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between Vue components
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Write concise, technical TypeScript code with accurate examples
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Avoid enums; use const objects instead
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statementsVue 3 app code in
src/renderer/srcshould be organized intocomponents/,stores/,views/,i18n/,lib/directories with shell UI insrc/renderer/shell/
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/**
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Use lowercase with dashes for directories (e.g., components/auth-wizard)
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching
Leverage ref, reactive, and computed for reactive state management
Use provide/inject for dependency injection when appropriate
Use Iconify/Vue for icon implementation
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/renderer/src/**/*.{ts,tsx,vue}: Use TypeScript with Vue 3 Composition API for the renderer application
All user-facing strings must use vue-i18n keys insrc/renderer/src/i18n
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/renderer/src/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
src/renderer/src/components/**/*.vue: Use Tailwind for styles in Vue components
Vue component files must use PascalCase naming (e.g.,ChatInput.vue)
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
src/**/*.{ts,tsx,vue,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with single quotes, no semicolons, and 100 character width
Files:
src/renderer/src/components/message/MessageBlockToolCall.vue
🧠 Learnings (1)
📚 Learning: 2025-11-25T05:26:43.510Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-11-25T05:26:43.510Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Avoid hardcoding user-facing text and ensure all user-visible text uses the i18n translation system
Applied to files:
src/renderer/src/components/message/MessageBlockToolCall.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
* fix: Multiple Issues Affecting Conversation Flow and Model Settings (#1166) * fix: #1164 support maxtoken 2 * fix: mcp tool panel close btn #1163 * fix: #1162 file content in converation * fix(search-assistant): exclude acp models * fix: #1072 thinkcontent respects the global font size set * feat: Hebrew (he-IL) Translation (#1157) * feat: Hebrew (he-IL) Translation * feat: add workspace view to acp agents (#1158) * feat: add workspaceview for acp agent * feat: support workspace dirs * fix: workspace file refresh * fix: tool call status * fix: review refactor * chore: update readme * fix: add file context actions (#1160) * fix: keep last file list * feat(acp-workspace): add file context actions * fix(acp-workspace): move path helpers to preload * fix(preload): allow empty relative path * fix(preload): escape quotes when formatting paths * chore: update docs * fix: Multiple Issues Affecting Conversation Flow and Model Settings (#1166) * fix: #1164 support maxtoken 2 * fix: mcp tool panel close btn #1163 * fix: #1162 file content in converation * fix(search-assistant): exclude acp models * fix: #1072 thinkcontent respects the global font size set * feat: add new i18n translation * feat: add custom font setting (#1167) * feat(settings): add font customization controls * feat: change font with monaco * chore: remove log and remove unuse key * fix: linux font parse * feat: use font-list to get font * fix: font setting not work on settings page (#1169) * style: unify scroll bar style (#1173) * feat: acp init and process manage (#1171) * feat: init acp process on select * feat: warm up acp process on new thread * fix: reuse warmup process * fix: vue warning * chore: add plan for acp debug panel * feat: add debugview for acp * feat: add i18n for debug * fix: code review * fix: ai review * fix: Shutdown may skip releasing warmup‑only processes due to using warmupKey instead of agentId. * chore: update markdown renderer * chore: update mermaid node * Merge commit from fork * chore: update markstream-vue to version 0.0.3-beta.3 fix link renderer feat html_inline render * fix: increase button size for web content limit adjustment * fix: close app kill all acp processes (#1175) * fix: close app kill all acp processes * fix: disable tool call merge * fix: handle uncatch error * fix: remove redundant type * feat: add shell to powerpack (#1178) * chore(powerpack): randomize shell workdir * feat: exclusive inmem server in terminal display * fix: add sandbox in shell script --------- Co-authored-by: xiaomo <[email protected]> Co-authored-by: Simon He <[email protected]> Co-authored-by: Simon He <[email protected]>
Summary
Testing
Codex Task
Summary by CodeRabbit
New Features
Improvements
Bug Fixes / UI
✏️ Tip: You can customize this high-level summary in your review settings.