Skip to content

Conversation

@zerob13
Copy link
Collaborator

@zerob13 zerob13 commented Dec 12, 2025

Summary

  • generate a unique temporary working directory for shell commands under the app temp path to avoid reusing the process directory

Testing

  • pnpm run typecheck:node
  • pnpm run typecheck:web

Codex Task

Summary by CodeRabbit

  • New Features

    • Added shell command execution with cross-platform support (Windows PowerShell, macOS/Linux Bash)
    • Automatic environment detection for shell operations
  • Improvements

    • English user-facing messages and clearer prompts for code execution flows
    • More robust error handling and cleanup for shell command workflows
  • Bug Fixes / UI

    • Prevented terminal-style rendering for the shell tool in the Powerpack server to improve display consistency

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 12, 2025

Walkthrough

Adds 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 run_shell_command when served by Powerpack in the renderer.

Changes

Cohort / File(s) Summary
Powerpack shell runtime
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts
Adds ShellEnvironment type, host OS detection, shell selection logic, RunShellCommandArgsSchema, run_shell_command tool integration, executeShellCommand and ensureShellWorkdir implementations, shell working-directory init and fallback, stdout/stderr formatting, enhanced error handling/cleanup, and English user-facing text updates.
Renderer terminal guard
src/renderer/src/components/message/MessageBlockToolCall.vue
Adds guard to terminal-detection logic: when tool_name is run_shell_command and server_name (lowercased) is powerpack, isTerminalTool is forced false to avoid terminal rendering for that combination.

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review OS detection and shell-argument construction for Windows vs Unix
  • Verify input validation schema and injection/escaping considerations
  • Inspect working-directory creation/fallback and cross-platform path handling
  • Confirm error handling, resource cleanup, and integration with tool registry
  • Check renderer guard to ensure UI rendering logic remains consistent

Poem

🐰 I hopped into code, a shell to tend,
PowerShell on Windows, Bash to befriend;
I guarded the UI where terminals hide,
Ran commands, caught output, then cleaned with pride.
Hooray — a quiet rabbit-run, cross-platform ride! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'feat: add shell to powerpack' is partially related to the changeset but lacks precision. The PR objectives indicate the main focus is randomizing/generating unique temp directories for shell commands, not broadly adding shell support. The title omits this critical detail and uses the vague phrase 'add shell'. Refine the title to reflect the specific change: 'feat: generate unique temp directory for powerpack shell commands' or similar, clarifying the core objective.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/add-cross-platform-shell-execution-support

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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() on PowerpackServer to fs.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 over console.*.

Also applies to: 287-297

🧹 Nitpick comments (2)
src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts (2)

50-68: Tighten RunShellCommandArgsSchema to reduce foot-guns (timeout bounds, command length, and workdir constraints).
Right now any-length commands + any timeout + arbitrary workdir are 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; consider parsed.error.issues (or .message) to keep responses concise.
  • New/changed console.* logging should ideally be logger.* 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2cb8c41 and a6e0eb3.

📒 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.ts for main-to-renderer communication, broadcasting events via mainWindow.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 in src/main/ with presenters in presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider) and eventbus.ts for 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.ts after implementing them in inMemoryServers/

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 src directory

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)

Comment on lines +16 to +22
type ShellEnvironment = {
platform: 'windows' | 'mac' | 'linux'
shellName: 'powershell' | 'bash'
shellExecutable: string
buildArgs: (command: string) => string[]
promptHint: string
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines 155 to 178
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.'
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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 -200

Repository: 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.ts

Repository: 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.ts

Repository: 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 -100

Repository: 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 -50

Repository: 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 3

Repository: 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 5

Repository: 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 -40

Repository: 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.ts

Repository: 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.

Suggested change
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.'
}
}

Comment on lines 261 to 285
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'
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Comment on lines 384 to 393
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)
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.ts

Repository: 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 -20

Repository: 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.ts

Repository: 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.

@zerob13 zerob13 changed the title Randomize powerpack shell temp directory feat: add shell to powerpack Dec 12, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between a6e0eb3 and a1c2293.

📒 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 src directory

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 statements

Vue 3 app code in src/renderer/src should be organized into components/, stores/, views/, i18n/, lib/ directories with shell UI in src/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 in src/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)

@zerob13 zerob13 merged commit d92ef64 into dev Dec 12, 2025
1 check passed
zerob13 added a commit that referenced this pull request Dec 12, 2025
* 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]>
@zerob13 zerob13 deleted the codex/add-cross-platform-shell-execution-support branch December 13, 2025 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants