feat: implement keeb/shell model for shell command execution

[keeb]

Summary

• Add keeb/shell model type for executing shell commands on the host system
• Captures stdout, stderr, and exit code from command execution
• Supports optional workingDir, timeout, and env attributes

Test plan

• Unit tests for input/resource schema validation
• Unit tests for execute method (simple commands, pipes, complex commands)
• Unit tests for error handling (command failures, nonexistent commands)
• Manual test with deno run dev model create/method run

🤖 Generated with Claude Code

[github-actions]

PR Review: feat: implement keeb/shell model for shell command execution

Summary

This PR adds a new keeb/shell model type for executing shell commands. The implementation follows the established model patterns in the codebase and includes comprehensive test coverage.

---

✅ No Blocking Issues

The code looks good and is ready to merge. Below are some suggestions for consideration.

---

Suggestions (Non-Blocking)

1. Security Consideration: Command Injection Awareness

The shell model accepts arbitrary commands from the input and executes them via sh -c. While this is the intended functionality and there's no direct user input vulnerability (the commands come from model inputs), consider adding documentation that warns users about the security implications of running arbitrary shell commands, especially if inputs can be dynamically generated.

2. Timeout Implementation Edge Case (shell_model.ts:89-107)

The timeout implementation uses Promise.race with child.kill("SIGTERM"). A few considerations:

• If the process doesn't respond to SIGTERM, it will remain as a zombie process. Consider following up with SIGKILL after a grace period.
• The timeout error is thrown but then caught by the outer try/catch, which sets exitCode = -1. This is reasonable but may lose the "timed out" distinction in some cases.

3. Missing Timeout Test

While there are comprehensive tests for the execute method, there's no test for the timeout functionality. Consider adding a test like:

Deno.test("shellModel.methods.execute times out slow commands", async () => {
  const input = ModelInput.create({
    name: "test-shell",
    attributes: {
      run: "sleep 10",
      timeout: 100,
    },
  });
  const result = await shellModel.methods.execute.execute(input, {
    repoDir: "/tmp",
  });
  assertEquals(result.resource.attributes.exitCode, -1);
  assertStringIncludes(result.resource.attributes.stderr as string, "timed out");
});

4. Environment Variables Inheritance

The current implementation only sets env when explicitly provided. This means if attrs.env is provided, the command runs with ONLY those env vars, not inheriting from the parent process. This might be intentional for isolation, but it could surprise users. Consider documenting this behavior or using { ...Deno.env.toObject(), ...attrs.env } if inheritance is desired.

5. Minor: Type Assertion in Tests (shell_model_test.ts:285, 298)

The tests use as string and as number type assertions:

const durationMs = result.resource.attributes.durationMs as number;
assertStringIncludes(result.resource.attributes.stderr as string, ...);

These are fine for test code, but the resource attributes could be typed more strongly if the schema validation guarantees the types.

---

Positive Notes

1. Clean architecture: Follows existing model patterns (echo_model.ts) consistently
2. Comprehensive test coverage: 28 tests covering schema validation, command execution, error handling, and various options
3. Good error handling: Graceful handling of command failures, non-existent commands, and validation errors
4. Proper use of Zod schemas: Well-documented schemas with appropriate constraints
5. Named exports: Follows the CLAUDE.md requirement for named exports
6. TypeScript strict mode compliant: No any types, proper type annotations

---

LGTM! 🎉