diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml new file mode 100644 index 00000000..a4a2c73a --- /dev/null +++ b/.github/workflows/python.yml @@ -0,0 +1,128 @@ +# CI for bashkit Python bindings +# Builds the native extension via maturin and runs pytest on each PR. +# Complements publish-python.yml (release-only) with per-PR validation. + +name: Python + +on: + push: + branches: [main] + paths: + - "crates/bashkit-python/**" + - "crates/bashkit/**" + - "Cargo.toml" + - "Cargo.lock" + - ".github/workflows/python.yml" + pull_request: + branches: [main] + paths: + - "crates/bashkit-python/**" + - "crates/bashkit/**" + - "Cargo.toml" + - "Cargo.lock" + - ".github/workflows/python.yml" + workflow_call: + +permissions: + contents: read + +env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 + +jobs: + lint: + name: Lint & Format + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: astral-sh/setup-uv@v7 + + - name: Ruff check + run: uvx ruff check crates/bashkit-python + + - name: Ruff format + run: uvx ruff format --check crates/bashkit-python + + test: + name: Test (Python ${{ matrix.python-version }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.9", "3.12", "3.13"] + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-python@v6 + with: + python-version: ${{ matrix.python-version }} + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - uses: Swatinem/rust-cache@v2 + + - name: Build wheel + uses: PyO3/maturin-action@v1 + with: + command: build + args: --release --out dist -i python${{ matrix.python-version }} + rust-toolchain: stable + working-directory: crates/bashkit-python + + - name: Install wheel and test dependencies + run: | + pip install bashkit --no-index --find-links crates/bashkit-python/dist --force-reinstall + pip install pytest pytest-asyncio + + - name: Run tests + working-directory: crates/bashkit-python + run: pytest tests/ -v + + # Verify wheel builds and passes twine check + build-wheel: + name: Build wheel + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-python@v6 + with: + python-version: "3.12" + + - name: Build wheel + uses: PyO3/maturin-action@v1 + with: + command: build + args: --release --out dist + rust-toolchain: stable + working-directory: crates/bashkit-python + + - name: Verify wheel metadata + run: | + pip install twine + twine check crates/bashkit-python/dist/* + + - uses: actions/upload-artifact@v6 + with: + name: python-wheel + path: crates/bashkit-python/dist + retention-days: 5 + + # Gate job for branch protection + python-check: + name: Python Check + if: always() + needs: [lint, test, build-wheel] + runs-on: ubuntu-latest + steps: + - name: Verify all jobs passed + run: | + if [[ "${{ needs.lint.result }}" != "success" ]] || \ + [[ "${{ needs.test.result }}" != "success" ]] || \ + [[ "${{ needs.build-wheel.result }}" != "success" ]]; then + echo "One or more Python CI jobs failed" + exit 1 + fi diff --git a/AGENTS.md b/AGENTS.md index 6a5480f6..d2168617 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -85,6 +85,14 @@ just pre-pr # Pre-PR checks - `cargo fmt` and `cargo clippy -- -D warnings` - License checks: `cargo deny check` (see `deny.toml`) +### Python + +- Python bindings in `crates/bashkit-python/` +- Linter/formatter: `ruff` (config in `pyproject.toml`) +- `ruff check crates/bashkit-python` and `ruff format --check crates/bashkit-python` +- Tests: `pytest crates/bashkit-python/tests/ -v` (requires `maturin develop` first) +- CI: `.github/workflows/python.yml` (lint, test on 3.9/3.12/3.13, build wheel) + ### Pre-PR Checklist 1. `just pre-pr` (runs 2-4 automatically) @@ -100,6 +108,7 @@ just pre-pr # Pre-PR checks 11. Resolve all PR comments 12. `cargo bench --bench parallel_execution` if touching Arc/async/Interpreter/builtins (see `specs/007-parallel-execution.md`) 13. `just bench` if changes might impact performance (interpreter, builtins, tools) +14. `ruff check crates/bashkit-python && ruff format --check crates/bashkit-python` if touching Python code ### CI diff --git a/crates/bashkit-python/bashkit/_bashkit.pyi b/crates/bashkit-python/bashkit/_bashkit.pyi index 4c24f92a..ce9380e9 100644 --- a/crates/bashkit-python/bashkit/_bashkit.pyi +++ b/crates/bashkit-python/bashkit/_bashkit.pyi @@ -1,14 +1,12 @@ """Type stubs for bashkit_py native module.""" -from typing import Optional - class ExecResult: """Result from executing bash commands.""" stdout: str stderr: str exit_code: int - error: Optional[str] + error: str | None success: bool def to_dict(self) -> dict[str, any]: ... @@ -33,10 +31,10 @@ class BashTool: def __init__( self, - username: Optional[str] = None, - hostname: Optional[str] = None, - max_commands: Optional[int] = None, - max_loop_iterations: Optional[int] = None, + username: str | None = None, + hostname: str | None = None, + max_commands: int | None = None, + max_loop_iterations: int | None = None, ) -> None: """Create a new BashTool instance. diff --git a/crates/bashkit-python/bashkit/deepagents.py b/crates/bashkit-python/bashkit/deepagents.py index 607ded54..327029fc 100644 --- a/crates/bashkit-python/bashkit/deepagents.py +++ b/crates/bashkit-python/bashkit/deepagents.py @@ -15,7 +15,7 @@ import uuid from datetime import datetime, timezone -from typing import Optional, TYPE_CHECKING +from typing import TYPE_CHECKING from bashkit import BashTool as NativeBashTool @@ -25,14 +25,14 @@ # Check for deepagents availability try: from deepagents.backends.protocol import ( - SandboxBackendProtocol, + EditResult, ExecuteResponse, + FileDownloadResponse, FileInfo, + FileUploadResponse, GrepMatch, - EditResult, + SandboxBackendProtocol, WriteResult, - FileDownloadResponse, - FileUploadResponse, ) from langchain.agents.middleware.types import AgentMiddleware from langchain_core.tools import tool as langchain_tool @@ -84,11 +84,11 @@ class BashkitMiddleware(AgentMiddleware): def __init__( self, - bash_tool: Optional[NativeBashTool] = None, - username: Optional[str] = None, - hostname: Optional[str] = None, - max_commands: Optional[int] = None, - max_loop_iterations: Optional[int] = None, + bash_tool: NativeBashTool | None = None, + username: str | None = None, + hostname: str | None = None, + max_commands: int | None = None, + max_loop_iterations: int | None = None, ): """Initialize middleware. @@ -128,7 +128,6 @@ def reset(self) -> None: if self._owns_bash: self._bash.reset() - class BashkitBackend(SandboxBackendProtocol): """Backend implementing SandboxBackendProtocol with Bashkit VFS. @@ -147,10 +146,10 @@ class BashkitBackend(SandboxBackendProtocol): def __init__( self, - username: Optional[str] = None, - hostname: Optional[str] = None, - max_commands: Optional[int] = None, - max_loop_iterations: Optional[int] = None, + username: str | None = None, + hostname: str | None = None, + max_commands: int | None = None, + max_loop_iterations: int | None = None, ): self._bash = NativeBashTool( username=username, @@ -189,7 +188,7 @@ def read(self, file_path: str, offset: int = 0, limit: int = 2000) -> str: if result.exit_code != 0: return f"Error: {result.stderr or 'File not found'}" lines = result.stdout.splitlines() - selected = lines[offset:offset + limit] + selected = lines[offset : offset + limit] return "\n".join(f"{i:6d}\t{line}" for i, line in enumerate(selected, start=offset + 1)) async def aread(self, file_path: str, offset: int = 0, limit: int = 2000) -> str: @@ -213,11 +212,16 @@ def edit(self, file_path: str, old_string: str, new_string: str, replace_all: bo return EditResult(error="old_string not found") if count > 1 and not replace_all: return EditResult(error=f"Found {count} times. Use replace_all=True") - new_content = content.replace(old_string, new_string) if replace_all else content.replace(old_string, new_string, 1) + if replace_all: + new_content = content.replace(old_string, new_string) + else: + new_content = content.replace(old_string, new_string, 1) wr = self.write(file_path, new_content) return EditResult(error=wr.error, path=file_path) - async def aedit(self, file_path: str, old_string: str, new_string: str, replace_all: bool = False) -> EditResult: + async def aedit( + self, file_path: str, old_string: str, new_string: str, replace_all: bool = False + ) -> EditResult: return self.edit(file_path, old_string, new_string, replace_all) # === File Discovery === @@ -234,12 +238,16 @@ def ls_info(self, path: str) -> list[FileInfo]: name = " ".join(parts[8:]) if name in (".", ".."): continue - files.append(FileInfo( - path=f"{path.rstrip('/')}/{name}", name=name, - is_dir=parts[0].startswith("d"), - size=int(parts[4]) if parts[4].isdigit() else 0, - created_at=_now_iso(), modified_at=_now_iso(), - )) + files.append( + FileInfo( + path=f"{path.rstrip('/')}/{name}", + name=name, + is_dir=parts[0].startswith("d"), + size=int(parts[4]) if parts[4].isdigit() else 0, + created_at=_now_iso(), + modified_at=_now_iso(), + ) + ) return files async def als_info(self, path: str) -> list[FileInfo]: @@ -251,8 +259,16 @@ def glob_info(self, pattern: str, path: str = "/") -> list[FileInfo]: if result.exit_code != 0: return [] return [ - FileInfo(path=p.strip(), name=p.strip().split("/")[-1], is_dir=False, size=0, created_at=_now_iso(), modified_at=_now_iso()) - for p in result.stdout.splitlines() if p.strip() + FileInfo( + path=p.strip(), + name=p.strip().split("/")[-1], + is_dir=False, + size=0, + created_at=_now_iso(), + modified_at=_now_iso(), + ) + for p in result.stdout.splitlines() + if p.strip() ] async def aglob_info(self, pattern: str, path: str = "/") -> list[FileInfo]: @@ -273,7 +289,9 @@ def grep_raw(self, pattern: str, path: str | None = None, glob: str | None = Non continue return matches - async def agrep_raw(self, pattern: str, path: str | None = None, glob: str | None = None) -> list[GrepMatch] | str: + async def agrep_raw( + self, pattern: str, path: str | None = None, glob: str | None = None + ) -> list[GrepMatch] | str: return self.grep_raw(pattern, path, glob) # === File Transfer === @@ -285,7 +303,9 @@ def download_files(self, paths: list[str]) -> list[FileDownloadResponse]: if result.exit_code == 0: responses.append(FileDownloadResponse(path=p, content=result.stdout.encode(), error=None)) else: - responses.append(FileDownloadResponse(path=p, content=None, error=result.stderr or "File not found")) + responses.append( + FileDownloadResponse(path=p, content=None, error=result.stderr or "File not found") + ) return responses async def adownload_files(self, paths: list[str]) -> list[FileDownloadResponse]: @@ -316,14 +336,14 @@ def reset(self) -> None: self._bash.reset() -def create_bash_middleware(**kwargs) -> "BashkitMiddleware": +def create_bash_middleware(**kwargs) -> BashkitMiddleware: """Create BashkitMiddleware for Deep Agents.""" if not DEEPAGENTS_AVAILABLE: raise ImportError("deepagents required. Install: pip install 'bashkit[deepagents]'") return BashkitMiddleware(**kwargs) -def create_bashkit_backend(**kwargs) -> "BashkitBackend": +def create_bashkit_backend(**kwargs) -> BashkitBackend: """Create BashkitBackend for Deep Agents.""" if not DEEPAGENTS_AVAILABLE: raise ImportError("deepagents required. Install: pip install 'bashkit[deepagents]'") diff --git a/crates/bashkit-python/bashkit/langchain.py b/crates/bashkit-python/bashkit/langchain.py index d6ad50dc..6fd861fe 100644 --- a/crates/bashkit-python/bashkit/langchain.py +++ b/crates/bashkit-python/bashkit/langchain.py @@ -14,9 +14,6 @@ from __future__ import annotations -import asyncio -from typing import Optional, Type - try: from langchain_core.tools import BaseTool, ToolException from pydantic import BaseModel, Field, PrivateAttr @@ -40,9 +37,7 @@ def PrivateAttr(*args, **kwargs): class BashToolInput(BaseModel): """Input schema for BashTool.""" - commands: str = Field( - description="Bash commands to execute (like `bash -c 'commands'`)" - ) + commands: str = Field(description="Bash commands to execute (like `bash -c 'commands'`)") if LANGCHAIN_AVAILABLE: @@ -58,7 +53,7 @@ class BashkitTool(BaseTool): name: str = "" # Set in __init__ from bashkit description: str = "" # Set in __init__ from bashkit - args_schema: Type[BaseModel] = BashToolInput + args_schema: type[BaseModel] = BashToolInput handle_tool_error: bool = True # Internal state - use PrivateAttr for pydantic v2 compatibility @@ -66,10 +61,10 @@ class BashkitTool(BaseTool): def __init__( self, - username: Optional[str] = None, - hostname: Optional[str] = None, - max_commands: Optional[int] = None, - max_loop_iterations: Optional[int] = None, + username: str | None = None, + hostname: str | None = None, + max_commands: int | None = None, + max_loop_iterations: int | None = None, **kwargs, ): """Initialize BashkitTool. @@ -126,11 +121,11 @@ async def _arun(self, commands: str) -> str: def create_bash_tool( - username: Optional[str] = None, - hostname: Optional[str] = None, - max_commands: Optional[int] = None, - max_loop_iterations: Optional[int] = None, -) -> "BashkitTool": + username: str | None = None, + hostname: str | None = None, + max_commands: int | None = None, + max_loop_iterations: int | None = None, +) -> BashkitTool: """Create a LangChain-compatible Bashkit tool. Args: @@ -152,8 +147,7 @@ def create_bash_tool( """ if not LANGCHAIN_AVAILABLE: raise ImportError( - "langchain-core is required for LangChain integration. " - "Install with: pip install 'bashkit[langchain]'" + "langchain-core is required for LangChain integration. Install with: pip install 'bashkit[langchain]'" ) return BashkitTool( diff --git a/crates/bashkit-python/bashkit/pydantic_ai.py b/crates/bashkit-python/bashkit/pydantic_ai.py index 3b4d4721..be6ffd25 100644 --- a/crates/bashkit-python/bashkit/pydantic_ai.py +++ b/crates/bashkit-python/bashkit/pydantic_ai.py @@ -13,8 +13,6 @@ from __future__ import annotations -from typing import Optional - try: from pydantic_ai import Tool @@ -26,11 +24,11 @@ def create_bash_tool( - username: Optional[str] = None, - hostname: Optional[str] = None, - max_commands: Optional[int] = None, - max_loop_iterations: Optional[int] = None, -) -> "Tool": + username: str | None = None, + hostname: str | None = None, + max_commands: int | None = None, + max_loop_iterations: int | None = None, +) -> Tool: """Create a PydanticAI Tool wrapping Bashkit. Args: @@ -53,8 +51,7 @@ def create_bash_tool( """ if not PYDANTIC_AI_AVAILABLE: raise ImportError( - "pydantic-ai is required for PydanticAI integration. " - "Install with: pip install 'bashkit[pydantic-ai]'" + "pydantic-ai is required for PydanticAI integration. Install with: pip install 'bashkit[pydantic-ai]'" ) native = NativeBashTool( diff --git a/crates/bashkit-python/pyproject.toml b/crates/bashkit-python/pyproject.toml index 5a1e2795..107cb5ef 100644 --- a/crates/bashkit-python/pyproject.toml +++ b/crates/bashkit-python/pyproject.toml @@ -37,5 +37,15 @@ features = ["pyo3/extension-module"] python-source = "." module-name = "bashkit._bashkit" +[tool.ruff] +target-version = "py39" +line-length = 120 + +[tool.ruff.lint] +select = ["E", "F", "W", "I", "UP"] + +[tool.ruff.lint.isort] +known-first-party = ["bashkit"] + [tool.pytest.ini_options] asyncio_mode = "auto" diff --git a/crates/bashkit-python/tests/test_bashkit.py b/crates/bashkit-python/tests/test_bashkit.py new file mode 100644 index 00000000..a1daa1e3 --- /dev/null +++ b/crates/bashkit-python/tests/test_bashkit.py @@ -0,0 +1,181 @@ +"""Tests for bashkit Python bindings.""" + +import json + +import pytest + +from bashkit import BashTool, create_langchain_tool_spec + +# -- Construction ----------------------------------------------------------- + + +def test_default_construction(): + tool = BashTool() + assert tool.name == "bashkit" + assert isinstance(tool.short_description, str) + assert isinstance(tool.version, str) + + +def test_custom_construction(): + tool = BashTool(username="alice", hostname="box", max_commands=100, max_loop_iterations=500) + assert repr(tool) == 'BashTool(username="alice", hostname="box")' + + +# -- Sync execution --------------------------------------------------------- + + +def test_echo(): + tool = BashTool() + r = tool.execute_sync("echo hello") + assert r.exit_code == 0 + assert r.stdout.strip() == "hello" + assert r.stderr == "" + assert r.error is None + assert r.success is True + + +def test_exit_code(): + tool = BashTool() + r = tool.execute_sync("exit 42") + assert r.exit_code == 42 + assert r.success is False + + +def test_stderr(): + tool = BashTool() + r = tool.execute_sync("echo err >&2") + assert "err" in r.stderr + + +def test_multiline(): + tool = BashTool() + r = tool.execute_sync("echo a; echo b; echo c") + assert r.exit_code == 0 + lines = r.stdout.strip().splitlines() + assert lines == ["a", "b", "c"] + + +def test_state_persists(): + """Filesystem and variables persist across calls.""" + tool = BashTool() + tool.execute_sync("export FOO=bar") + r = tool.execute_sync("echo $FOO") + assert r.stdout.strip() == "bar" + + +def test_file_persistence(): + """Files created in one call are visible in the next.""" + tool = BashTool() + tool.execute_sync("echo content > /tmp/test.txt") + r = tool.execute_sync("cat /tmp/test.txt") + assert r.stdout.strip() == "content" + + +# -- Async execution -------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_async_execute(): + tool = BashTool() + r = await tool.execute("echo async_hello") + assert r.exit_code == 0 + assert r.stdout.strip() == "async_hello" + + +@pytest.mark.asyncio +async def test_async_state_persists(): + tool = BashTool() + await tool.execute("X=123") + r = await tool.execute("echo $X") + assert r.stdout.strip() == "123" + + +# -- ExecResult ------------------------------------------------------------- + + +def test_exec_result_to_dict(): + tool = BashTool() + r = tool.execute_sync("echo hi") + d = r.to_dict() + assert d["stdout"].strip() == "hi" + assert d["exit_code"] == 0 + assert d["stderr"] == "" + assert d["error"] is None + + +def test_exec_result_repr(): + tool = BashTool() + r = tool.execute_sync("echo hi") + assert "ExecResult" in repr(r) + + +def test_exec_result_str_success(): + tool = BashTool() + r = tool.execute_sync("echo ok") + assert str(r).strip() == "ok" + + +def test_exec_result_str_failure(): + tool = BashTool() + r = tool.execute_sync("exit 1") + assert "Error" in str(r) + + +# -- Reset ------------------------------------------------------------------ + + +def test_reset(): + tool = BashTool() + tool.execute_sync("export KEEP=1") + tool.reset() + r = tool.execute_sync("echo ${KEEP:-empty}") + assert r.stdout.strip() == "empty" + + +# -- LLM metadata ---------------------------------------------------------- + + +def test_description(): + tool = BashTool() + desc = tool.description() + assert isinstance(desc, str) + assert len(desc) > 0 + + +def test_help(): + tool = BashTool() + h = tool.help() + assert isinstance(h, str) + assert len(h) > 0 + + +def test_system_prompt(): + tool = BashTool() + sp = tool.system_prompt() + assert isinstance(sp, str) + assert len(sp) > 0 + + +def test_input_schema(): + tool = BashTool() + schema = tool.input_schema() + parsed = json.loads(schema) + assert "type" in parsed or "properties" in parsed + + +def test_output_schema(): + tool = BashTool() + schema = tool.output_schema() + parsed = json.loads(schema) + assert "type" in parsed or "properties" in parsed + + +# -- LangChain tool spec --------------------------------------------------- + + +def test_langchain_tool_spec(): + spec = create_langchain_tool_spec() + assert "name" in spec + assert "description" in spec + assert "args_schema" in spec + assert spec["name"] == "bashkit" diff --git a/justfile b/justfile index b1a6882f..07a52ee0 100644 --- a/justfile +++ b/justfile @@ -32,6 +32,11 @@ check: cargo clippy --all-targets -- -D warnings cargo test +# Lint and format-check Python bindings +python-lint: + ruff check crates/bashkit-python + ruff format --check crates/bashkit-python + # Run all pre-PR checks pre-pr: check vet @echo "Pre-PR checks passed" diff --git a/specs/013-python-package.md b/specs/013-python-package.md index 06f51e77..9eac2e02 100644 --- a/specs/013-python-package.md +++ b/specs/013-python-package.md @@ -19,6 +19,8 @@ crates/bashkit-python/ │ ├── langchain.py # LangChain integration │ ├── deepagents.py # Deep Agents integration │ └── pydantic_ai.py # PydanticAI integration +└── tests/ + └── test_bashkit.py # Pytest suite for bindings ``` ## Build System @@ -151,6 +153,33 @@ pip install bashkit[pydantic-ai] # + pydantic-ai pip install bashkit[dev] # + pytest, pytest-asyncio ``` +## CI + +File: `.github/workflows/python.yml` + +Runs on push to main and PRs (path-filtered to `crates/bashkit-python/`, `crates/bashkit/`, +`Cargo.toml`, `Cargo.lock`). + +``` +PR / push to main + ├── lint (ruff check + ruff format --check) + ├── test (maturin develop + pytest, Python 3.9/3.12/3.13) + ├── build-wheel (maturin build + twine check) + └── python-check (gate job for branch protection) +``` + +## Linting + +- **Linter/formatter**: [ruff](https://docs.astral.sh/ruff/) (config in `pyproject.toml`) +- **Rules**: E (pycodestyle), F (pyflakes), W (warnings), I (isort), UP (pyupgrade) +- **Target**: Python 3.9, line-length 120 + +```bash +ruff check crates/bashkit-python # lint +ruff format --check crates/bashkit-python # format check +ruff format crates/bashkit-python # auto-format +``` + ## Local Development ```bash @@ -158,7 +187,10 @@ cd crates/bashkit-python pip install maturin maturin develop # debug build, installs into current venv maturin develop --release # optimized build -pytest # run tests (needs dev extras) +pip install pytest pytest-asyncio +pytest tests/ -v # run tests +ruff check . # lint +ruff format . # format ``` ## Design Decisions