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 {


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 {


addressed in e3f9c76

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

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


addressed in e3f9c76

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

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


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>

github-actions · 2026-08-09T10:10:55Z

This pull request has been marked as stale because of no activity in the last 15 days. It will be closed in the next 5 days unless it is tagged "no stalebot" or other activity occurs.

brandonkeung · 2026-08-13T14:47:49Z

bump (prevent auto close PR)

mesutoezdil

some questions and small findings from a close read.

mesutoezdil · 2026-08-24T17:05:43Z

+			}
+		}
+
+		info, err := entry.Info()


wrong size for a symlink. shows symlink size, not file size.

mesutoezdil · 2026-08-24T17:05:43Z

+		// one as a file, so skip it rather than treating it as an error.
+		return walkEntrySkip, ""
+	}
+	if !fi.Mode().IsRegular() {


read_file can hang on a fifo. no fix like grep_file's.

mesutoezdil · 2026-08-24T17:05:43Z

+            # is_file() follows symlinks and checks S_ISREG, so this also
+            # excludes FIFOs/sockets/devices -- opening one for reading can
+            # block indefinitely (e.g. a FIFO with no writer connected).
+            if not entry.is_file():


broken symlink skipped silently. no skip count here.

mesutoezdil · 2026-08-24T17:05:43Z

+                # Skip entries whose symlink-resolved target escapes the
+                # root being searched, so a symlink can't be used to read
+                # files outside the requested directory.
+                safe_entry = _validate_path(entry, allowed_root)


allowed_root too wide. symlink can escape this folder.

mesutoezdil · 2026-08-24T17:05:43Z

+		for scanner.Scan() {
+			if line := scanner.Text(); re.MatchString(line) {
+				if len(line) > 2000 {
+					line = line[:2000] + "..."


cuts by byte. can break utf-8.

mesutoezdil · 2026-08-24T17:05:43Z

+
+	info, err := os.Stat(path)
+	if err != nil {
+		return "", err


error not wrapped. line above wraps it.

mesutoezdil · 2026-08-24T17:05:43Z

 	}

-	sessionRoot, err := filepath.Abs(sessionPath)
+	sessionRoot, err := filepath.EvalSymlinks(sessionPath)


three functions repeat same steps. share one function?

mesutoezdil · 2026-08-24T17:05:43Z

+        # separately-nullable attributes) keeps "both present or both
+        # absent" a structural guarantee instead of a convention the two
+        # attributes have to be kept in sync by hand.
+        self._file_search_tools: list[BaseTool] = (


bash always added. go skips it. match go?

mesutoezdil · 2026-08-24T17:05:43Z

+				return fmt.Sprintf("Error searching %s: %v", strings.TrimSpace(in.Path), err), nil
+			}
+
+			content, err := skillruntime.GrepContent(path, in.Pattern, in.Recursive, in.IgnoreCase)


no timeout here. python has 30s.

mesutoezdil · 2026-08-24T17:05:43Z

+// Also registered (separately, for `kagent env` CLI discoverability only,
+// not read here) as KagentEnableFileSearchTools in go/core/pkg/env/kagent.go
+// -- keep both string literals in sync if this name ever changes.
+const enableFileSearchToolsEnv = "KAGENT_ENABLE_FILE_SEARCH_TOOLS"


same string in two files. share one constant?

mesutoezdil · 2026-08-24T17:08:41Z

resolve conflicts pls

…counting Addresses mesutoezdil's review findings on PR kagent-dev#2267. Six were real bugs; all are regression-tested (each new test verified to fail against the pre-fix code). Go: - ListDirContent reported a symlink's own size -- the byte length of its stored target path -- because entry.Info() is Lstat-based. It now stats the target, so a symlinked file reports the file's size and a broken link is listed bare, matching Python's pathlib behavior. The needed os.Stat result was already being computed and discarded. - ReadFileContent had no regular-file guard, so read_file on a FIFO with no writer blocked forever with no timeout on the path. It now rejects non-regular files, as GrepContent already did. - Line truncation sliced bytes, not runes. Beyond emitting invalid UTF-8 from a split sequence, it cut CJK text at ~668 characters rather than the 2000 the tool descriptions promise. Both sites now share a truncateRunes helper that cuts on a rune boundary, matching Python's per-code-point slicing. - Wrap the bare errors in GrepContent and ReadFileContent with %w. Python: - grep_content dropped broken symlinks silently. A dangling link is a genuine read failure, so it now counts toward the "N entries could not be read" annotation, matching Go's walkEntryUnreadable. FIFOs and sockets stay silent, matching walkEntrySkip. - Entries were validated only against allowed_root, which in production is the whole session dir plus the skills dir -- wider than the directory being searched. A symlink could therefore pull in a sibling the caller never asked about, contradicting both the tool description and the README. Entries are now also bounded by the search root, as Go already does. Also corrects comments in five places that described list_files/grep_file as opt-in "alongside bash". Upstream removed bash's gating entirely in kagent-dev#2498, so bash is now unconditional in both runtimes and that comparison was false. Adds a test pinning the KAGENT_ENABLE_FILE_SEARCH_TOOLS literal in go/adk/pkg/tools to the `kagent env` registry entry in go/core/pkg/env so the two cannot drift. The import is test-only and does not add a dependency from the agent runtime onto the control-plane module. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

grep_file's scanner set a 1MB line buffer; ReadFileContent kept bufio's 64KB default. A file with one longer line -- a minified bundle, a single-line JSON blob -- therefore failed read_file outright, losing every other line in the file, while grep_file handled the same file fine and read_file's own tool description promises such lines are truncated. Python truncated correctly throughout, so this was a Go-only regression introduced alongside grep_file. Extract scanFileLines as the single reader behind both. It owns the non-regular-file rejection (previously duplicated), the buffer cap, and error wrapping, so the two paths can no longer drift on any of the three. Lines past maxLineBytes still error rather than truncate: uncapping would mean buffering an arbitrarily long line in a sandbox reading untrusted files. Also in this change: - Wrap ListDirContent's os.ReadDir error, the last bare return beside a wrapped one. - Correct file_search_tools_enabled()'s docstring, which still claimed list_files/grep_file are "disabled by default, same as bash". Bash's gate was removed upstream in kagent-dev#2498; this was the last of six such sites. - Drop the _validate_path call in grep_content's entry loop. The search-root bound added beside it is strictly narrower, and file_or_dir_path is already validated against allowed_root, so the wider check is dead. - Give Python the _MAX_LINE_CHARS/_truncate_line pair Go already had, replacing four repetitions of the literal 2000. - Express the three path resolvers as pathPolicy values. allowSkillsRoot had been picking the denial message as a side effect, which would misdescribe any future resolver that denied the skills root for a reason other than writability. - Fold TestResolveReadPath_AllowsSymlinkedSkillsDirectory and TestResolveWritePath_BlocksSkillsSymlink into TestResolvePathContainment, which already covered both cells of that matrix. - Correct the grep_file call site's no-timeout rationale, which cited RE2's linearity -- an answer about the match, not the walk that GrepContent's own doc comment identifies as the unbounded part. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

Pure move -- no logic changes. The bodies are byte-identical to their previous location except for three added comment lines on WithinRoot, noting that resolveSandboxedPath is its second caller. shell.go is named for shell execution but had accumulated a directory-walking search engine: walkEntryAction, classifyWalkEntry, WithinRoot, and GrepContent share no state and no callers with CommandExecutor or the file primitives. shell_test.go had also reached 1152 lines, 446 of them TestGrepContent alone. shell.go 492 -> 308 grep.go (new) 197 shell_test.go 1152 -> 704 grep_test.go (new) 460 shell.go drops its regexp import and shell_test.go its errors import; both were used only by the moved code. WithinRoot moves with grep rather than staying behind, where it would have had no local caller left. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

CI runs `ruff format --diff .` and fails on any diff. Neither line this branch added had been through the formatter -- only `ruff check`, which passes on both. The prompts.py change joins an implicitly-concatenated string literal; the resulting bash description is byte-identical, verified against the exact expected suffix. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

scanFileLines asked bufio for a fixed 64KB initial buffer. That size was never deliberate: it arrived in 27fb17c while addressing a review ask to *cap* grep's line length, where the 1MB maximum was the point and the initial size was incidental. Consolidating the two readers then spread it from grep_file to read_file as well, so every file read paid it too. A nil initial buffer keeps the same 1MB cap -- bufio grows from 4KB by doubling -- so long lines are unaffected, verified with a 900,000-char line reading back intact. Isolating just the buffer argument: fixed 64KB 18820 ns/op 65746 B/op nil (grows) 10978 ns/op 4297 B/op Roughly 15x less allocated per file. It matters because grep_file walks whole trees: a 10,000-file search was allocating ~650MB of transient buffer to read files that are mostly a few KB. Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

Go decides how to treat each entry of a recursive grep in a named function with three named outcomes -- classifyWalkEntry returning walkEntryGrep, walkEntrySkip or walkEntryUnreadable. Python made the same decision inline, held together by comments that referenced the Go symbols by name: # matching the split Go makes in classifyWalkEntry: # ... as Go's walkEntryUnreadable does. # ... as Go's walkEntrySkip does. That is a cross-runtime contract enforced by English. Renaming the Go function would silently falsify it -- no test fails, nothing rebuilds. It matters here specifically because runtime drift is this feature's recurring defect: six of the ten findings in the last review were the two implementations having quietly diverged. Extract _classify_walk_entry with the same three outcomes, the same argument order and the same return shape, so the two can be diffed by reading them side by side rather than by trusting prose. The entry loop drops from 38 lines (25 of them comment, 65%) to 14 (3 comment, 21%), and the reasoning moves onto the function it actually describes. The docstring is explicit that the two are not branch-for-branch identical and should not be forced to be -- Go tests IsDir, EvalSymlinks, Stat and IsRegular separately because WalkDir hands it directories and unresolvable links, while os.walk yields only filenames and Path.is_file() collapses those cases into one call. It also records the one divergence we know of and could not construct: Go counts a failed Stat on a non-symlink as UNREADABLE, where Path.is_file() swallows the OSError and reaches SKIP. Behavior is unchanged. Each outcome is independently pinned, verified by inverting it and confirming the suite catches it: UNREADABLE -> SKIP (broken links uncounted) caught SKIP -> GREP (escaping symlinks searched) caught GREP -> SKIP (regular files never read) caught SKIP -> UNREADABLE (FIFOs counted) caught Adds a direct table test for the classifier, which the inline form could not have: the three-way split is now assertable without going through grep_content, including the symlink-loop case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: brandonkeung <brandonlkeung@gmail.com>

classifyWalkEntry had no direct test -- grep for it in *_test.go and the only hit was inside a comment. Its three-way split was covered indirectly through TestGrepContent, so a change to the classifier surfaced as an integration assertion failing somewhere downstream rather than as the specific case that broke. That asymmetry undercut the previous commit. Python got both the extracted classifier and a table test asserting all its outcomes; Go had the structure but nothing equivalent to compare against, which is precisely the side-by-side reading the two are supposed to support. Same cases, same order, same expected outcomes as test_classify_walk_entry_covers_each_outcome in kagent-skills. The directory case has no Python counterpart on purpose, and says so: filepath.WalkDir hands this function directories while os.walk yields only filenames, so only Go can reach it. Verified it guards the boundary rather than just passing: disabling the WithinRoot check makes symlink_escaping_the_root fail with the resolved out-of-root path it would have leaked. 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 Outdated

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

github-actions Bot added the stale This issue or PR has become stale label Aug 9, 2026

Merge branch 'kagent-dev:main' into feat/grep-list-files-tools

bd346e9

github-actions Bot removed the stale This issue or PR has become stale label Aug 14, 2026

brandonkeung requested review from EItanya and mesutoezdil August 24, 2026 16:18

mesutoezdil reviewed Aug 24, 2026

View reviewed changes

brandonkeung and others added 9 commits August 26, 2026 11:19

Merge upstream/main into feat/grep-list-files-tools

c94bf09

test: cover long line and symlink fixes through ADK tool path

3c45fe9

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 • 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!

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!

github-actions Bot commented Aug 9, 2026

Uh oh!

brandonkeung commented Aug 13, 2026

Uh oh!

mesutoezdil left a comment

Choose a reason for hiding this comment

Uh oh!

mesutoezdil Aug 24, 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!

mesutoezdil Aug 24, 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!

mesutoezdil Aug 24, 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!

mesutoezdil Aug 24, 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!

mesutoezdil Aug 24, 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 commented Jul 15, 2026 •

edited

Loading

mesutoezdil Jul 21, 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

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading

mesutoezdil Aug 24, 2026 •

edited

Loading