minimal shell tool (#191)

packages/server/src/tools/shell.json

@@ -2,16 +2,16 @@
   "type": "object",
   "properties": {
     "command": {
-      "description": "The exact bash command or sequence of commands (using ';' or '&&') to execute. Must adhere to usage guidelines. Example: 'npm install && npm run build'",
+      "description": "Exact bash command to execute as `bash -c <command>`",
       "type": "string"
     },
     "description": {
-      "description": "Optional: A brief, user-centric explanation of what the command does and why it's being run. Used for logging and confirmation prompts. Example: 'Install project dependencies'",
+      "description": "Brief description of the command for the user. Be specific and concise. Ideally a single sentence. Can be up to 3 sentences for clarity. No line breaks.",
       "type": "string"
     },
-    "runInBackground": {
-      "description": "If true, execute the command in the background using '&'. Defaults to false. Use for servers or long tasks.",
-      "type": "boolean"
+    "directory": {
+      "description": "(OPTIONAL) Directory to run the command in, if not the project root directory. Must be relative to the project root directory and must already exist.",
+      "type": "string"
     }
   },
   "required": ["command"]

packages/server/src/tools/shell.md

@@ -1 +1,11 @@
-This is a minimal shell tool.
+This is a minimal shell tool that executes a given command as `bash -c <command>`.
+Command can be any valid single-line Bash command.
+The following information is returned:
+
+Command: Given command.
+Stdout: Output on stdout stream. Can be `(empty)` or partial on error.
+Stderr: Output on stderr stream. Can be `(empty)` or partial on error.
+Error: Error or `(none)` if no error occurred.
+Exit Code: Exit code or `(none)` if terminated by signal.
+Signal: Signal number or `(none)` if no signal was received.
+Background PIDs: List of background processes started or `(none)`.

packages/server/src/tools/shell.ts

@@ -5,6 +5,7 @@
  */
 
 import fs from 'fs';
+import path from 'path';
 import { Config } from '../config/config.js';
 import {
   BaseTool,
@@ -14,16 +15,17 @@ import {
   ToolConfirmationOutcome,
 } from './tools.js';
 import toolParameterSchema from './shell.json' with { type: 'json' };
-
+import { SchemaValidator } from '../utils/schemaValidator.js';
 export interface ShellToolParams {
   command: string;
   description?: string;
+  directory?: string;
 }
+import { spawn } from 'child_process';
 
 export class ShellTool extends BaseTool<ShellToolParams, ToolResult> {
   static Name: string = 'execute_bash_command';
   private readonly config: Config;
-  private cwd: string;
   private whitelist: Set<string> = new Set();
 
   constructor(config: Config) {
@@ -37,29 +39,71 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> {
       toolParameterSchema,
     );
     this.config = config;
-    this.cwd = config.getTargetDir();
   }
 
   getDescription(params: ShellToolParams): string {
-    return params.description || `Execute \`${params.command}\` in ${this.cwd}`;
+    let description = `${params.command}`;
+    if (params.description) {
+      // replace any line breaks with spaces, in case instructions are not followed
+      description += ` (${params.description.replace(/\n/g, ' ')})`;
+    }
+    if (params.directory) {
+      description += ` @ ${params.directory}`;
+    }
+    return description;
   }
 
-  validateToolParams(_params: ShellToolParams): string | null {
-    // TODO: validate the command here
+  getCommandRoot(command: string): string | undefined {
+    return command
+      .trim() // remove leading and trailing whitespace
+      .replace(/[{}()]/g, '') // remove all grouping operators
+      .split(/[\s;&|]+/)[0] // split on any whitespace or separator or chaining operators and take first part
+      ?.split(/[/\\]/) // split on any path separators (or return undefined if previous line was undefined)
+      .pop(); // take last part and return command root (or undefined if previous line was empty)
+  }
+
+  validateToolParams(params: ShellToolParams): string | null {
+    if (
+      !SchemaValidator.validate(
+        this.parameterSchema as Record<string, unknown>,
+        params,
+      )
+    ) {
+      return `Parameters failed schema validation.`;
+    }
+    if (!params.command.trim()) {
+      return 'Command cannot be empty.';
+    }
+    if (params.command.match(/[^\S ]/)) {
+      return 'Command cannot contain any whitespace other than plain spaces.';
+    }
+    if (!this.getCommandRoot(params.command)) {
+      return 'Could not identify command root to obtain permission from user.';
+    }
+    if (params.directory) {
+      if (path.isAbsolute(params.directory)) {
+        return 'Directory cannot be absolute. Must be relative to the project root directory.';
+      }
+      const directory = path.resolve(
+        this.config.getTargetDir(),
+        params.directory,
+      );
+      if (!fs.existsSync(directory)) {
+        return 'Directory must exist.';
+      }
+    }
     return null;
   }
 
   async shouldConfirmExecute(
     params: ShellToolParams,
   ): Promise<ToolCallConfirmationDetails | false> {
-    const rootCommand =
-      params.command
-        .trim()
-        .split(/[\s;&&|]+/)[0]
-        ?.split(/[/\\]/)
-        .pop() || 'unknown';
+    if (this.validateToolParams(params)) {
+      return false; // skip confirmation, execute call will fail immediately
+    }
+    const rootCommand = this.getCommandRoot(params.command)!; // must be non-empty string post-validation
     if (this.whitelist.has(rootCommand)) {
-      return false;
+      return false; // already approved and whitelisted
     }
     const confirmationDetails: ToolExecuteConfirmationDetails = {
       title: 'Confirm Shell Command',
@@ -74,10 +118,89 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> {
     return confirmationDetails;
   }
 
-  async execute(_params: ShellToolParams): Promise<ToolResult> {
+  async execute(params: ShellToolParams): Promise<ToolResult> {
+    const validationError = this.validateToolParams(params);
+    if (validationError) {
+      return {
+        llmContent: [
+          `Command rejected: ${params.command}`,
+          `Reason: ${validationError}`,
+        ].join('\n'),
+        returnDisplay: `Error: ${validationError}`,
+      };
+    }
+
+    // wrap command to append subprocess pids (via pgrep) to stderr
+    let command = params.command.trim();
+    if (!command.endsWith('&')) command += ';';
+    command = `{ ${command} }; { echo __PGREP__; pgrep -g 0; echo __DONE__; } >&2`;
+
+    // spawn command in specified directory (or project root if not specified)
+    const shell = spawn('bash', ['-c', command], {
+      stdio: ['ignore', 'pipe', 'pipe'],
+      detached: true, // ensure subprocess starts its own process group (esp. in Linux)
+      cwd: path.resolve(this.config.getTargetDir(), params.directory || ''),
+    });
+
+    let stdout = '';
+    let output = '';
+    shell.stdout.on('data', (data: Buffer) => {
+      stdout += data.toString();
+      output += data.toString();
+    });
+
+    let stderr = '';
+    let pgrepStarted = false;
+    const backgroundPIDs: number[] = [];
+    shell.stderr.on('data', (data: Buffer) => {
+      if (data.toString().trim() === '__PGREP__') {
+        pgrepStarted = true;
+      } else if (data.toString().trim() === '__DONE__') {
+        shell.stdout.destroy();
+        shell.stderr.destroy();
+      } else if (pgrepStarted) {
+        // allow multiple lines and exclude shell's own pid (esp. in Linux)
+        for (const line of data.toString().trim().split('\n')) {
+          const pid = Number(line.trim());
+          if (pid !== shell.pid) {
+            backgroundPIDs.push(pid);
+          }
+        }
+      } else {
+        stderr += data.toString();
+        output += data.toString();
+      }
+    });
+
+    let error: Error | null = null;
+    shell.on('error', (err: Error) => {
+      error = err;
+    });
+
+    let code: number | null = null;
+    let signal: NodeJS.Signals | null = null;
+    shell.on(
+      'close',
+      (_code: number | null, _signal: NodeJS.Signals | null) => {
+        code = _code;
+        signal = _signal;
+      },
+    );
+
+    // wait for the shell to exit
+    await new Promise((resolve) => shell.on('close', resolve));
+
     return {
-      llmContent: 'hello',
-      returnDisplay: 'hello',
+      llmContent: [
+        `Command: ${params.command}`,
+        `Stdout: ${stdout || '(empty)'}`,
+        `Stderr: ${stderr || '(empty)'}`,
+        `Error: ${error ?? '(none)'}`,
+        `Exit Code: ${code ?? '(none)'}`,
+        `Signal: ${signal ?? '(none)'}`,
+        `Background PIDs: ${backgroundPIDs.length ? backgroundPIDs.join(', ') : '(none)'}`,
+      ].join('\n'),
+      returnDisplay: output,
     };
   }
 }