feat: add shell tool and tests by notowen333 · Pull Request #39 · strands-agents/devtools

notowen333 · 2026-03-06T19:27:06Z

Issue #, if available:
https://github.com/strands-agents/private-sdk-python-staging/issues/340

Description of changes:

simple subprocess based shell avoiding full terminal emulation with pty

Distinct from TS:

persistent shell (i.e. cd and export will persist across tool calls)
binary encoded output is parsed
standard error and standard out write to the same pipe to avoid race conditions
clean garbage collection tied to agent lifecycle
uses default shell instead of specifying bash

First PR to add pytest tests to the python scripts section of the repo, so needed to add __init__ and requirements.txt

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

zastrowm · 2026-03-06T20:29:37Z

strands-command/scripts/python/tests/integ/test_shell_tool.py

+    """Test basic shell command execution through agent."""
+    result = agent("Run: echo 'hello'")
+    result_str = str(result)
+    print(f"\nBasic execution result: {result_str[:300]}")


Can we add validation that the tool is being called; I think the easiest way is to validate the messages on the agent?

added tool call validation on all the tests

zastrowm · 2026-03-06T20:30:56Z

strands-command/scripts/python/tests/integ/test_shell_tool.py

+    print("\n--- Complex Multi-Step Workflow Test ---")
+
+    # Step 1: Create temporary directory structure
+    result1 = agent("Create a temp directory at /tmp/shell_test_$$ and cd into it, then confirm with pwd")


Use a temp directory and instruct it to write there: https://github.com/strands-agents/sdk-python/blob/2d766c4e6974732590439a79ab10c4f07f2d1806/tests_integ/test_invalid_tool_names.py#L10

Then can we assert the actual directory instead of taking the agent output.

In general I want to verify the actual behavior instead of assuming that the agent output text indicating that it did it

Makes sense fixed on both counts

I made similar fixes for the remaining tests to use the temp directory to verify instead of the agent response string.

zastrowm · 2026-03-06T20:32:06Z

strands-command/scripts/python/tests/test_shell_tool.py

+from ..shell_tool import ShellSession, shell_tool
+
+
+class TestShellSession:


We don't have it written anywhere that I can find) but we just do top level functions for tests; no classes

sure flattened

zastrowm · 2026-03-06T20:33:17Z

strands-command/scripts/python/shell_tool.py

+
+    def __init__(self, timeout: int = 30):
+        self.timeout = timeout
+        self.process = None


Can timeout and process be private?

zastrowm · 2026-03-06T21:16:44Z

strands-command/scripts/python/shell_tool.py

+                self._alive = False
+                self._buffer_condition.notify_all()
+
+    def run(self, command: str, timeout: int | None = None) -> str:


Should we always have some sort of default timeout? I'm a little concerned about commands taht hang the agent- but maybe that's not our concern?

I noticed today that Kiro timedout (though noticed that it was still running) - this reinforces my belief of a default timeout. That might move us to a class-based tool so that we can also allow folks to specify the default timeout

The default timeout is 30 seconds see above code snippet. Is it not enough to accept timeout arg in the tool call? If this is the only configuration a class based tool seems like overkill for the pattern.

zastrowm · 2026-03-06T21:17:17Z

strands-command/scripts/python/tests/integ/__init__.py

Can you add to the PR description how this compares to the TS implementation?

most of the traits I noted are the differentiators. Separated it out in the description.

zastrowm · 2026-03-06T21:18:30Z

strands-command/scripts/python/shell_tool.py

+    Uses the system default shell ($SHELL, defaulting to /bin/bash) with clean
+    startup configuration (--noprofile --norc for bash, -f for zsh).
+
+    **Supported commands:**


Do we really need this? Listing specific commands seems odd when we're not providing them

I removed the specific supported commands but left unsupported

zastrowm · 2026-03-06T21:19:22Z

strands-command/scripts/python/shell_tool.py

+    if restart and (not command or command.strip() == ""):
+        if hasattr(tool_context.agent, SESSION_ATTR):
+            getattr(tool_context.agent, SESSION_ATTR).stop()
+        setattr(tool_context.agent, SESSION_ATTR, ShellSession())


What is this doing? Are we modifying the agent's properties?

We should find another way of doing this. (this could indicate a gap in the SDK FWIW)

This is storing a ShellSession on the agent object itself for persistence.

Something like this would be the main alternative:

_sessions = {} # Module-level @tool(context=True) def shell_tool(command: str, ..., tool_context: ToolContext = None): agent_id = id(tool_context.agent) # Use memory address as key if agent_id not in _sessions: _sessions[agent_id] = ShellSession() return _sessions[agent_id].run(command)

notowen333 requested review from Unshure, afarntrog and zastrowm March 6, 2026 19:27

zastrowm requested changes Mar 6, 2026

View reviewed changes

feat: add shell tool and tests

db37c44

notowen333 force-pushed the maintainer/notowen333-shell-tool branch from 98bd484 to db37c44 Compare March 9, 2026 15:36

		from ..shell_tool import ShellSession, shell_tool


		class TestShellSession:

Conversation

notowen333 commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

notowen333 Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

notowen333 commented Mar 6, 2026 •

edited

Loading

notowen333 Mar 9, 2026 •

edited

Loading