feat: Add environment tools matching Python parity - #555
Open
AmaadMartin wants to merge 3 commits into
Open
Conversation
AmaadMartin
force-pushed
the
feat/js-environment-tools
branch
2 times, most recently
from
July 28, 2026 21:18
5e87d4d to
4124503
Compare
added 3 commits
July 28, 2026 18:53
Correctness:
- escapeRegExp escaped nothing: the character class closed early, so
old_string was matched as a live regex. Metacharacters could match the
wrong bytes and an unbalanced group threw out of runAsync instead of
returning the {status: 'error'} contract.
- EditFile expanded $&, $` and $1 in new_string. Use a function
replacement so the text is inserted literally.
- The truncation notice reported the original length as if it were the
limit ("truncated to 10 chars" for 5 characters of content).
Types (no suppressions anywhere in the diff):
- Each tool declares and exports its result type (ExecuteResult,
ReadFileResult, WriteFileResult, EditFileResult), narrowing the
Promise<unknown> from BaseTool. Callers and tests are typed with no
annotations.
- catch blocks bind unknown and narrow through toError /
isFileNotFoundError instead of `catch (e: any)`; the exec failure path
uses child_process.ExecException, which already declares killed, code,
stdout and stderr.
- isValidLineNumber is a module-level type predicate, replacing an inline
closure, a dead !isNaN check and two unchecked casts on unknown.
Security:
- Execute now passes through the standard tool-confirmation gate before
it shells out, matching how RunSkillInlineScriptTool gates
model-provided code. workingDir is not a boundary for a shell command,
and these tools are public API.
Also: @experimental on all four tool classes (they are public API and
adk-python decorates all four), drop the leading underscore from the
description constant, and document that path containment is lexical and
does not resolve symlinks.
AmaadMartin
force-pushed
the
feat/js-environment-tools
branch
from
July 29, 2026 02:07
4124503 to
c3360f4
Compare
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
N/A — no existing issue tracks this work.
2. Or, if no issue exists, describe the change:
Problem:
The JS variant of ADK lacks parity with the
adk-pythonenvironment tools, missingnecessary functionality for agent local file manipulation and shell execution.
Solution:
Added Node.js parity environment tools (
ExecuteTool,ReadFileTool,WriteFileTool,EditFileTool) and a bundledEnvironmentToolsetincore/src/tools/environment/. The toolset also injects theENVIRONMENT_INSTRUCTIONinto the LLM system requests for bounded contextualusage.
One deliberate deviation from
adk-python, please read:ExecuteToolrequires an explicit tool confirmation before it runs a command.adk-python's execute tool has no such gate, but it also does not run anythingitself: it delegates to
BaseEnvironment.execute(), so "run on the host" is onebackend an embedder selects. This port collapses that seam into a hardcoded
child_processimplementation behindworkingDir: string, which makes hostexecution the only option — a model-authored string becomes arbitrary code
execution on the machine running the agent, and
cwdis not a boundary. The gateuses the same mechanism
RunSkillInlineScriptToolalready uses onmainformodel-provided scripts. Restoring the
BaseEnvironmentseam would be the betterlong-term answer and is a reasonable follow-up.
All four tool classes carry
@experimental, matchingadk-pythonand reflectingthat they became public API here via
core/src/common.tsandcore/src/index.ts.Each tool exports its result type (
ExecuteResult,ReadFileResult,WriteFileResult,EditFileResult) rather than returning the base class'sPromise<unknown>.There are no
any,as any,eslint-disable,@ts-ignoreor@ts-expect-erroruses anywhere in this diff, in
src/or in the tests.Testing Plan
Unit Tests:
Added
core/test/tools/environment/tools_test.ts, covering boundaries acrossreading, writing, editing and shell execution, plus the confirmation gate and
literal-text matching in
EditFile. Local run:Manual End-to-End (E2E) Tests:
npm run build.npx vitest run --project unit:core core/test/tools/environment/tools_test.tsto exercise the tools against a temporary working directory.
Checklist
Additional context
resolveAndValidatePathperforms a lexical containment check. It rejects..and absolute-path arguments, but
path.resolve/path.relativenever touch thefilesystem, so a symlink inside
workingDirthat points elsewhere still resolvesthrough. That is intentional — reading through symlinks is necessary for real
workspace layouts such as the package links npm creates under
node_modules— andthe docstring says so explicitly. It is not a sandbox; callers who need one should
point the tools at an isolated filesystem.
This branch also carries one small, independent fix commit:
fix(dev): output agent bundle in project .adk_build_cache for hoisted dependencies,which changes
dev/src/utils/agent_loader.tsto emit the compiled agent bundle intoa project-local
.adk_build_cachedirectory instead of the system temp directory, sothat hoisted
node_modulesdependencies resolve correctly. Happy to split this into aseparate PR if reviewers prefer.