feat: add list_files and grep_file skills tools - #2267

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 <[email protected]>

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 <[email protected]>

…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 <[email protected]>

…s-tools

Signed-off-by: brandonkeung <[email protected]>

# Conflicts:
#	python/packages/kagent-adk/src/kagent/adk/tools/__init__.py

_classify_walk_entry called Path.is_file() unguarded. That swallows ENOENT,
ENOTDIR, EBADF and ELOOP, but raises on EACCES -- so a stat failure escaped
the classifier and propagated out of grep_content, discarding every match
already found elsewhere in the tree.

The reachable case is a directory that is readable but not searchable (mode
0o444): os.walk lists its children fine, then stat on each child is denied.
The suite's existing permission tests use mode 0o000, where os.walk itself
fails and the error is already handled by the walk_errors path, so they
could not catch this.

Classify any OSError from those stats as UNREADABLE instead, matching the Go
runtime, whose classifyWalkEntry already maps every EvalSymlinks/Stat failure
to walkEntryUnreadable. The caller counts UNREADABLE and keeps walking.

Also corrects the docstring, which claimed this divergence was "not reachable
through any case we could construct". EACCES reaches it trivially. The
remaining divergence is narrower and is now described accurately.

Reported by EItanya in review of kagent-dev#2267.

Signed-off-by: brandonkeung <[email protected]>

Two defects in GrepFileTool, combined because the test for the second
depends on the harness the first one forced.

1. The 30s timeout could not fire.

   The match ran in a ThreadPoolExecutor, but `re` does not release the GIL
   while matching. A catastrophic pattern therefore froze every thread in the
   interpreter, the event loop included, so asyncio.wait_for never got
   scheduled to time anything out. Reproduced with `(a+)+$` against 32 "a"s
   plus "!": a 0.5s timeout did not fire and an independent 50ms heartbeat
   stopped dead until the process was killed. The tool's error message
   ("pattern took too long to match") was therefore unreachable in exactly
   the case it described.

   The match now runs in a single-use worker process, which has its own GIL,
   so the loop stays responsive and the deadline is honored. Unlike a thread,
   the runaway can also be killed, and it is -- otherwise the deadline would
   bound only the caller's wait while the worker kept burning a core.

   A per-call pool rather than a shared one: a shared pool saves ~100ms of
   spawn per call but forces a timeout to recycle shared state, killing
   workers belonging to unrelated greps and surfacing BrokenProcessPool to
   callers that did nothing wrong. At ~100ms inside a multi-second model turn
   that trade is not worth making.

2. The pattern was stripped before use.

   args.get("pattern", "").strip() turned "foo " into "foo", which matches
   "foobar" -- text the caller's pattern excludes. Whitespace is meaningful
   in a regex. Trim only to decide emptiness and search with what was asked
   for, which is the split skills.go already makes, so the two runtimes no
   longer disagree on identical input.

The existing timeout test faked slowness with time.sleep, which *releases*
the GIL, so it passed while the real hazard went unguarded. It now runs a
genuinely pathological match and asserts the event loop kept ticking. A
second, fast test asserts the work leaves this process, because the
behavioral test hangs rather than fails if the offload regresses to a thread
and there is no pytest-timeout plugin here to cut it short.

Tests that patched grep_content were rewritten against real files: a
MagicMock cannot be pickled to a worker process, and the child imports the
real module regardless.

Reported by EItanya in review of kagent-dev#2267.

Signed-off-by: brandonkeung <[email protected]>

Signed-off-by: brandonkeung <[email protected]>

…s-tools