feat: add list_files and grep_file skills tools by brandonkeung · Pull Request #2267 · kagent-dev/kagent

brandonkeung · 2026-07-15T20:56:31Z

Summary

Adds native, in-process list_files and grep_file tools to both the Go (go/adk/pkg/skills, go/adk/pkg/tools) and Python (kagent-skills, kagent-adk) agent runtimes, alongside the existing read_file/write_file/edit_file/bash skills tools.
Gives agents safe, non-privileged file visibility without depending on bash, which some deployments disable for privilege/security reasons.
Hardened across several review passes; fixes along the way:
- Go: a symlink-resolution inconsistency in resolveReadPath/resolveWritePath/resolveEditPath could reject valid paths under a symlinked session root.
- Go: NewSkillsTools failed entirely (dropping all tools) when the bash command executor couldn't be constructed, instead of just omitting bash.
- Go: filepath.WalkDir root resolution fixed so an unresolved symlink root is actually recursed into; an err-shadowing bug that silently truncated results on a WalkDir failure fixed.
- Both: grep_file no longer hangs indefinitely on a FIFO (or other non-regular file) with no writer connected, in both the recursive walk and single-target paths.
- Both: matched lines are capped to 2000 chars (matching read_file's existing convention) — previously unbounded in Python, and could fail the whole search past 1MB in Go.
- Python: GrepFileTool now uses a dedicated thread pool instead of the shared default pool, since a hung regex match can't be forcibly killed and would otherwise starve unrelated work.
- Both: a single unreadable file or subdirectory is now a skip rather than an abort of the whole search, so one bad entry doesn't discard matches already found elsewhere in the tree — annotated as "no matches found (N entries could not be read)" so a systemic failure isn't indistinguishable from a genuinely empty search.
- Both: a fully unreadable search root now surfaces a real error instead of a misleadingly confident empty result.
- Per maintainer feedback, list_files/grep_file are now disabled by default and gated behind KAGENT_ENABLE_FILE_SEARCH_TOOLS (set on the Agent's env to opt in) — mirroring how bash is already effectively opt-in. read_file/write_file/edit_file/bash/skills are unaffected.

Test plan

Go: TestGrepContent (go/adk/pkg/skills/shell_test.go) — matches, recursion, symlink escape/resolution, FIFO hang safety, unreadable file/subdirectory/root handling, output truncation — and TestListFilesAndGrepFileTools_RunThroughADK, TestNewSkillsTools_OmitsBashWithoutSRTSettings (go/adk/pkg/tools/skills_test.go)
Python: unit tests in kagent-skills/kagent-adk covering the same scenarios (path traversal, recursive/ignore-case, symlink escape, FIFO hang safety, unreadable file/subdirectory/root handling, truncation, GrepFileTool timeout behavior)
All new regression tests verified to fail against the pre-fix code, confirming they exercise the actual bug
Verified end-to-end in a live kind cluster via the browser UI for both runtimes (dedicated Go- and Python-runtime test agents), including the FIFO-hang scenario and the unreadable-subdirectory/root scenarios, each invoking list_files/grep_file by name and returning correct output
Go/Python unit tests for the KAGENT_ENABLE_FILE_SEARCH_TOOLS flag's default-off/opt-in behavior (skills_test.go, test_skill_execution.py, new test_skills_plugin.py)
Verified end-to-end on a live kind cluster via A2A and the browser UI: dedicated Go- and Python-runtime test agents, with and without KAGENT_ENABLE_FILE_SEARCH_TOOLS set, confirming correct tool registration and that list_files/grep_file execute correctly when enabled

Copilot

Pull request overview

This PR extends Kagent’s agent toolsets (Go + Python runtimes) with native, in-process filesystem visibility tools—list_files and grep_file—so agents can inspect session/skills files without relying on bash (which may be disabled). It also adjusts Go path-resolution and tool construction behavior to be more robust around symlinked roots and missing sandbox-runtime settings.

Changes:

Added list_files/grep_file tool descriptions and implementations for both Python (kagent-skills, kagent-adk) and Go (go/adk/pkg/skills, go/adk/pkg/tools).
Updated Go tool initialization to omit bash when sandbox-runtime settings are unavailable, instead of failing the entire toolset.
Added/expanded unit tests covering new directory listing and grep behaviors across both runtimes.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
python/packages/kagent-skills/src/kagent/tests/unittests/test_skill_execution.py	Adds unit tests for `list_dir_content` / `grep_content` (including traversal/recursive/error cases).
python/packages/kagent-skills/src/kagent/skills/shell.py	Implements `list_dir_content` and `grep_content` core logic.
python/packages/kagent-skills/src/kagent/skills/prompts.py	Adds standardized prompt/description text for `list_files` and `grep_file`.
python/packages/kagent-skills/src/kagent/skills/init.py	Exposes new functions/descriptions in the public skills package API.
python/packages/kagent-adk/src/kagent/adk/tools/skills_toolset.py	Adds `ListFilesTool`/`GrepFileTool` to the default ADK toolset.
python/packages/kagent-adk/src/kagent/adk/tools/skills_plugin.py	Ensures agents get `list_files` / `grep_file` tools when missing.
python/packages/kagent-adk/src/kagent/adk/tools/file_tools.py	Introduces ADK tool wrappers `ListFilesTool` and `GrepFileTool` wired to skills implementations.
python/packages/kagent-adk/src/kagent/adk/tools/init.py	Exports the new tool classes.
go/adk/pkg/tools/skills.go	Adds Go `list_files`/`grep_file`, makes `bash` optional, and adjusts symlink root resolution.
go/adk/pkg/tools/skills_test.go	Adds tests for toolset composition and end-to-end tool invocation via `functiontool.Run()`.
go/adk/pkg/skills/shell.go	Adds Go implementations `ListDirContent` and `GrepContent`.
go/adk/pkg/skills/shell_test.go	Adds unit tests for the new Go directory listing and grep functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Adds native, in-process list_files and grep_file tools to both the Go and Python agent runtimes, alongside the existing read_file/write_file/ edit_file/bash skills tools. Gives agents safe, non-privileged file visibility without depending on bash, which some deployments disable for privilege/security reasons. Also fixes two related bugs found while implementing and testing this: - Go: a symlink-resolution inconsistency in resolveReadPath/ resolveWritePath/resolveEditPath could reject valid paths under a symlinked session root. - Go: NewSkillsTools failed entirely (dropping all tools) when the bash command executor couldn't be constructed, instead of omitting bash. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

- Skip symlinked entries that resolve outside the searched root during recursive grep_file, in both the Go and Python implementations. A symlink inside an otherwise-jailed directory (e.g. from an untrusted skill package) could previously be followed to read file contents outside the intended sandbox. - Bound the Go grep scanner's line buffer (was capped at the default 64KiB bufio.Scanner token size, which errored on long lines such as minified JSON). - Reject an empty path explicitly in the Go grep_file tool instead of surfacing a confusing "no file path provided" error from deeper in the call stack. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

Adds the two new tools to the quick-start import example and the Tool Workflow table, and notes the symlink-escape protection in the Security section, matching how the existing read_file/write_file/ edit_file/bash tools are already documented there. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

Thermos branch review turned up a real bug introduced by the previous symlink-escape fix, plus a consolidation opportunity and a missing timeout: - Go: `root, err := filepath.EvalSymlinks(path)` inside GrepContent's `if info.IsDir()` block shadowed the outer `err`, so a WalkDir failure was never observed by the `err != nil` check afterward. Combined with an in-bounds directory symlink (which WalkDir doesn't recurse into, and which grepFile can't read as a file), this caused the walk to abort silently partway through, returning a truncated "success" result with no error. Fixed by not shadowing err, and by explicitly skipping symlinked directories instead of letting grepFile fail on them. - Go: consolidated the symlink-escape containment check into a single shared `skillruntime.WithinRoot` helper (previously GrepContent had its own filepath.Rel-based check, duplicating the pre-existing isWithinRoot used by resolveReadPath/resolveEditPath/resolveWritePath) so there's one implementation of this security-relevant property instead of two that could drift. - Python: grep_file's regex match now runs via asyncio.to_thread with a 30s asyncio.wait_for timeout, mirroring the timeout bash already enforces. Python's re engine backtracks and a pathological, agent-controlled pattern run synchronously inside an async def could otherwise block the whole event loop indefinitely (Go is unaffected; its regexp package is RE2-based and linear-time). - Mention list_files/grep_file in the bash tool's own description in both languages, and log (debug level) when bash is omitted because the sandbox-runtime isn't configured, so its absence isn't silent. Verified live: both Go and Python runtime pods rebuilt and redeployed, confirmed via the UI that a recursive grep_file across a working directory containing the skills/ symlink (the exact scenario the shadowing bug silently broke) now correctly finds matches on both sides of the symlink. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

…d failures Across several review passes on grep_file/list_files, close out the remaining correctness gaps in the recursive-search path (Go and Python): - Fix filepath.WalkDir root resolution so an unresolved symlink root is actually recursed into, and fix an err-shadowing bug that silently truncated results on a WalkDir failure. - Skip non-regular files (FIFOs, sockets, devices) before opening them -- previously a FIFO with no writer connected would hang the search indefinitely, in both the recursive walk and single-target paths. - Cap matched lines to 2000 chars (matching read_file's existing convention); previously unbounded in Python and could fail the whole search past 1MB in Go. - Give GrepFileTool a dedicated thread pool in Python instead of the shared default pool, since a hung regex match can't be forcibly killed and would otherwise starve unrelated work. - Treat a single unreadable file or subdirectory as a skip rather than aborting the whole search, so one bad entry doesn't discard matches already found elsewhere in the tree. Annotate "no matches found (N entries could not be read)" when skips occurred, so a systemic failure isn't indistinguishable from a genuinely empty search. - Surface a real error, instead of a misleadingly confident empty result, when the search root itself is unreadable. - Extract Go's classifyWalkEntry and Python's _resolve_working_path helpers to keep the now-more-involved walk logic readable. Regression tests added for each fix above, verified to fail against the prior code. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

mesutoezdil · 2026-07-21T12:08:37Z

+	}
+
+	var result strings.Builder
+	for _, entry := range entries {


entry.isdir() does not follow symlinks, so the skills symlink in every session dir is listed as a plain file with the link size instead of skills/, stat the entry when it is a symlink like the python tool does.

addressed in e3f9c76

EItanya

Just out of curiosity, why do we need this? Can't it just use the shell tools? This is already in a sandbox so why would shell be disabled?

A maintainer asked that list_files/grep_file default to disabled rather than being registered unconditionally, since they give an agent broader filesystem visibility than read_file/write_file/edit_file. Both runtimes now check a single env var, KAGENT_ENABLE_FILE_SEARCH_TOOLS (off by default, same true-ish values "1"/"t"/"true" case-insensitive in both languages), before registering the two tools. read_file/write_file/ edit_file/skills/bash are unaffected. Verified end-to-end on a live kind cluster: dedicated Go- and Python-runtime test agents with and without the env var set, confirming the tools are absent/present in the registered tool list and functional when enabled. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

brandonkeung · 2026-07-23T20:53:43Z

Just out of curiosity, why do we need this? Can't it just use the shell tools? This is already in a sandbox so why would shell be disabled?

Some deployments turn bash off anyway for extra tightness (fewer arbitrary-command-execution surfaces even inside a sandbox), and list_files/grep_file were originally always-on regardless of that choice, which undercut it. Pushed a change gating both behind KAGENT_ENABLE_FILE_SEARCH_TOOLS (default off) so they follow the same opt-in posture as bash instead of being an end-run around it.

mesutoezdil · 2026-07-23T20:55:10Z

+				// A read error on one file (permission denied, a line
+				// exceeding the scan buffer, etc.) shouldn't abort matches
+				// already found elsewhere in the tree.
+				if grepErr := grepFile(p); grepErr != nil {


classifyWalkEntry checks the resolved symlink target but grepFile(p) right after reopens the raw path p not the resolved one, so if smth swaps the symlink in between it could still slip past root n get read, py version avoids this by grepping the resolved safe_entry instead

addressed in e3f9c76

mesutoezdil · 2026-07-23T20:55:17Z

+			}
+			return nil
+		})
+		if err == nil && skipped > 0 && result.Len() == 0 {


skipped count only shows up in the msg when result is totally empty, so if some files matched n others couldnt be read u never find out abt the skipped ones, kinda defeats the whole point of tracking it

addressed in e3f9c76

mesutoezdil · 2026-07-23T20:55:22Z

+        results.extend(grep_file(file_or_dir_path))
+
+    if not results:
+        if skipped:


same gap here too, skipped only shows up when results is totally empty, so a mix of some matches n some unreadable files just hides the skip count entirely

addressed in e3f9c76

mesutoezdil · 2026-07-23T20:58:03Z

2 more things i saw while reading

shell.go ReadFileContent (unrelated to this pr) never bumps the scanner buffer like GrepContent does right below it, so read_file just errors out completely on a file w one line over 64kb instead of truncating it
shell.go EditFileContent (also pre existing) only errors on ambiguous multi match when trimmed old_string is under 5 chars, but the py twin errors on any count>1 no matter the length, so edit_file acts diff depending on which runtime the agent is on

…list_files Addresses 4 issues mesutoezdil found in review on PR kagent-dev#2267: - ListDirContent listed a directory symlink (e.g. every session's "skills" entry) as a file instead of a directory, since entry.IsDir() doesn't follow symlinks. Now stats symlink entries to classify them correctly, matching Python's existing symlink-following behavior. - classifyWalkEntry verified a walked entry's resolved, in-bounds target, but grepFile then reopened the original unresolved path -- a verify-then-use gap where the symlink's target could differ between the check and the read. grepFile now reads the resolved path that was actually verified. This narrows the race but doesn't fully eliminate it (documented in a comment on classifyWalkEntry); closing it completely would need platform-specific work disproportionate to this file's existing security bar. - Go and Python both silently dropped the "N entries could not be read" note whenever there were also real matches, only surfacing it when the result was otherwise empty -- masking partial failures. Both now append it alongside real matches too. Verified end-to-end on a live kind cluster via A2A against redeployed Go- and Python-runtime test agents, extracting raw tool function_response payloads (not model-summarized text) to confirm each fix's actual behavior, plus a targeted regression check confirming symlink-escape protection still holds after the refactor. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

Copilot AI review requested due to automatic review settings July 15, 2026 20:56

brandonkeung requested review from a team and supreme-gg-gg as code owners July 15, 2026 20:56

github-actions Bot added the enhancement New feature or request label Jul 15, 2026

Copilot started reviewing on behalf of brandonkeung July 15, 2026 20:57 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread python/packages/kagent-skills/src/kagent/skills/shell.py Outdated

Comment thread go/adk/pkg/skills/shell.go Outdated

Comment thread go/adk/pkg/skills/shell.go

Comment thread go/adk/pkg/tools/skills.go Outdated

brandonkeung force-pushed the feat/grep-list-files-tools branch from 14ac799 to 7b55923 Compare July 15, 2026 21:01

github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Jul 16, 2026

brandonkeung added 5 commits July 16, 2026 15:07

brandonkeung force-pushed the feat/grep-list-files-tools branch from 8f8fb8c to 041e747 Compare July 16, 2026 19:08

mesutoezdil reviewed Jul 21, 2026

View reviewed changes

EItanya reviewed Jul 21, 2026

View reviewed changes

github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Jul 23, 2026

mesutoezdil reviewed Jul 23, 2026

View reviewed changes

Uh oh!

Conversation

brandonkeung commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mesutoezdil Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

brandonkeung Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

EItanya left a comment • 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!

brandonkeung commented Jul 23, 2026

Uh oh!

mesutoezdil Jul 23, 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!

brandonkeung Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mesutoezdil Jul 23, 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!

brandonkeung Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mesutoezdil Jul 23, 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!

brandonkeung Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mesutoezdil commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

brandonkeung commented Jul 15, 2026 •

edited

Loading

EItanya left a comment •

edited

Loading

mesutoezdil Jul 23, 2026 •

edited

Loading

mesutoezdil Jul 23, 2026 •

edited

Loading

mesutoezdil Jul 23, 2026 •

edited

Loading

mesutoezdil commented Jul 23, 2026 •

edited

Loading