feat(tool): add BashToolBuilder::configure for full BashBuilder access#2104
Open
chaliy wants to merge 2 commits into
Open
feat(tool): add BashToolBuilder::configure for full BashBuilder access#2104chaliy wants to merge 2 commits into
chaliy wants to merge 2 commits into
Conversation
BashToolBuilder re-declared a handful of BashBuilder knobs (username, hostname, limits, cwd, env, builtins) but couldn't reach the rest — network allowlist, mounts, hooks, git/ssh config, python/typescript, etc. — so tool users had strictly less power than Bash users (the 'inconsistent coverage' half of the duplication problem). Add BashToolBuilder::configure(|b: BashBuilder| ...), a thin-adapter escape hatch that runs against the fresh BashBuilder create_bash builds per execution, after the convenience setters so it can extend or override them. Steps are Arc<dyn Fn(BashBuilder) -> BashBuilder + Send + Sync> so the tool stays Clone and rebuilds a shell per call. The convenience fields stay (they also feed the help / system-prompt text). Also simplify input_schema / output_schema to call the static schema functions instead of reconstructing a throwaway builder.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | 3f151d7 | Commit Preview URL Branch Preview URL |
Jun 19 2026, 08:49 PM |
There was a problem hiding this comment.
Pull request overview
Adds an escape-hatch API to let BashToolBuilder apply arbitrary BashBuilder configuration on each execution, closing the feature gap between direct Bash::builder() usage and the tool wrapper.
Changes:
- Introduces
BashToolBuilder::configure(...)storingArc<dyn Fn(BashBuilder) -> BashBuilder + Send + Sync>steps and applying them last duringBashTool::create_bash(). - Simplifies
Tool::input_schema()/output_schema()forBashToolto return the static request/response schemas directly. - Adds a test asserting that
.configure()runs after convenience setters and can override them.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per review: help()/system_prompt() are generated from the convenience setters' stored fields, so values set only inside a configure() closure run but don't show up in the documentation text. Document the caveat and point users at the convenience setters for anything that should be documented.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Architectural item #2:
BashToolBuilderre-declared a handful ofBashBuilderknobs (username,hostname,limits,cwd,env,builtins) but couldn't reach the rest of theBashBuildersurface — network allowlist, mounts, hooks, git/ssh config,python/typescript, session/memory limits, etc. So tool users had strictly less power thanBashusers (the "inconsistent coverage" half of the duplication critique, and the more impactful one).The full-dedup alternative (route every setter through closures, drop the fields) doesn't work: the convenience fields are also read by the help / system-prompt text (it displays the configured username/hostname/limits/env), so they can't become opaque closures.
What
BashToolBuilder::configure(|b: BashBuilder| ...)— a thin-adapter escape hatch that runs against the freshBashBuildercreate_bashbuilds per execution, after the convenience setters so it can extend or override them. AnyBashBuildercapability is now reachable from a tool.Arc<dyn Fn(BashBuilder) -> BashBuilder + Send + Sync>soBashToolstaysCloneand rebuilds an isolated shell per call.input_schema/output_schemato call the static schema functions directly instead of reconstructing a throwawayBashToolBuilderfrom the tool's fields.The convenience setters/fields are unchanged (no breaking change; bindings/CLI unaffected).
Tests
test_configure_applies_to_underlying_bash_builder:.configure()sets env + overrides$USERvia.username()on the underlying builder, and runs after the convenience.env()— output confirms both apply and configure wins.bash_toolsuite green: 2491 lib unit tests + 948 integration tests. CLI builds; fmt + clippy clean.