feat(coder): File/CLI/Search mixins per §15.2

[kovtcharov]

Summary

Three foundational no-network mixins for `gaia-coder` (per §15.2 of `docs/plans/coder-agent.mdx`). Unblocks scaffold and downstream mixin work: the scaffold task wires these into `CoderAgent.init`; GitHub/Web/OSSReuse mixins build on top.

Sibling streams: `feature/gaia-coder-scaffold` (agent class + loop), `feature/gaia-coder-stores` (§15.1 SQLite stores).

What's in the box

• `FileToolsMixin` — `read_file`, `write_file`, `edit_file`, `search_code`, `glob`, `generate_diff`. Pure-Python (`re` + `pathlib` + `difflib`) so behaviour is deterministic on every platform. `read_file`/`edit_file` raise loudly on missing paths and non-unique matches per §2 principle 3.
• `CLIToolsMixin` — `run_cli_command`, `stop_process`, `list_processes`, `get_process_logs`, plus the static-denylist layer of the §6.8 guardrail stack (`rm -rf /`, `sudo`, `chmod 777`, `curl | bash`, `git push origin main`) and a local `ShellDeniedError` (§15.8 will fold it into a shared taxonomy). Background processes land in a module-level registry with reader threads draining stdio so tails don't deadlock on pipe buffers.
• `SearchToolsMixin` — `grep` (delegates to `search_code`), `find_symbol` (Python-only via `ast`), `list_files`. Non-Python `find_symbol` calls log WARN and return `[]` rather than silently skipping.

Test plan

• `pytest tests/coder/test_mixins.py` — 38/38 passing (happy + failure + registry per tool)
• `flake8 --max-line-length=88 --select=E9,F63,F7,F82,F401,F811 src/gaia/coder tests/coder` — clean
• `black --check` / `isort --check-only` — clean
• Integration smoke: `from gaia.coder.tools import *; FileToolsMixin().register_file_tools()` — 13 tools land in `_TOOL_REGISTRY`
• `git diff --stat` — changes confined to `src/gaia/coder/` and `tests/coder/`

Not in this PR

• `CoderAgent` class / `loop.py` — Stream A (`feature/gaia-coder-scaffold`)
• SQLite stores — Stream C (`feature/gaia-coder-stores`)
• `GitHubToolsMixin`, `WebToolsMixin`, `OSSReuseMixin` — follow-up tasks (require network)
• Full exception taxonomy for denylist — §15.8

Status: draft — do not merge until the scaffold PR lands and wiring is verified end-to-end.

[github-actions]

Code Review — #818 feat(coder): File/CLI/Search mixins per §15.2

Summary

Clean, well-tested foundation for the three no-network coder mixins. Stdlib-only implementation is portable and deterministic, the happy/failure/registration triad per tool is exemplary, and the 514-line test file is mostly readable. However, the post-rebase state of src/gaia/coder/__init__.py appears to drop the public re-exports added in #819 (CoderAgent, DEFAULT_LOOP, …), which breaks tests/coder/test_skeleton.py::test_package_imports. A handful of smaller issues around private-API coupling, a CLAUDE.md-forbidden silent except, and filesystem-walk hygiene are worth cleaning up in a follow-up.

Since the PR is already merged, flag these for a fast follow-up rather than a revert.

Issues Found

🔴 Critical

Package-level re-exports from #819 got dropped in the rebase (src/gaia/coder/__init__.py)

The diff replaces the __init__.py that #819 landed (which exposed CoderAgent, DEFAULT_LOOP, Loop, State, Transition) with a comment-only module. tests/coder/test_skeleton.py:50 still asserts those imports:

def test_package_imports() -> None:
    """Public re-exports must resolve from the top-level package."""
    from gaia.coder import CoderAgent  # noqa: F401  (import is the assertion)

Verified — after this PR's merge, gaia.coder.__init__ exports nothing. base.py:105 also has from gaia.coder import CoderAgent in its docstring example, so the docs are also wrong now.

Suggested fix — restore the exports while keeping the terser docstring from this PR:

# Copyright(C) 2026 Advanced Micro Devices, Inc. All rights reserved.
# SPDX-License-Identifier: MIT
"""gaia-coder — dev-tooling coding agent (see docs/plans/coder-agent.mdx).

This package is intentionally separate from ``src/gaia/agents/`` — the coder
is infrastructure for building GAIA, not a GAIA product agent.
"""

from gaia.coder.base import CoderAgent
from gaia.coder.loop import DEFAULT_LOOP, Loop, State, Transition

__all__ = [
    "CoderAgent",
    "DEFAULT_LOOP",
    "Loop",
    "State",
    "Transition",
]

🟡 Important

Silent except Exception: pass violates CLAUDE.md "No Silent Fallbacks" (src/gaia/coder/tools/cli.py:208)

finally:
    try:
        stream.close()
    except Exception:  # pylint: disable=broad-except
        pass

CLAUDE.md § No Silent Fallbacks — Fail Loudly explicitly calls this out and warns "The codebase has pre-existing except Exception: pass blocks… They are tech debt, not precedent". A stream-close failure here is real (e.g., OSError on a torn-down pipe) and swallowing it hides reader-thread bugs. Use a specific exception or re-raise with context:

        finally:
            try:
                stream.close()
            except OSError as e:
                logger.debug("reader thread: stream close failed (%s)", e)

Reaching into a private registry instead of the public accessor (src/gaia/coder/tools/search.py:672)

from gaia.agents.base.tools import _TOOL_REGISTRY
search_code = _TOOL_REGISTRY["search_code"]["function"]

The base module already ships a public helper at src/gaia/agents/base/tools.py:116:

def get_tool(tool_name: str) -> ...:
    """This is the public accessor for ``_TOOL_REGISTRY``.  Consumers outside
    [...] should use this function instead of importing ``_TOOL_REGISTRY``
    directly.
    """
    return _TOOL_REGISTRY.get(tool_name)

Use it:

            # Fetch search_code from the registry rather than re-importing so
            # tests that monkey-patch it still work.
            from gaia.agents.base.tools import get_tool

            entry = get_tool("search_code")
            if entry is None:
                raise RuntimeError(
                    "grep requires search_code to be registered; "
                    "call register_file_tools() first"
                )
            search_code = entry["function"]

Denylist is trivially bypassable AND prone to false positives (src/gaia/coder/tools/cli.py:146-164)

_SHELL_DENYLIST: tuple = ("rm -rf /", "sudo", "chmod 777", "curl | bash", "git push origin main")

Literal-substring matching against a joined argv:

• sudo matches benign commands like sudoku, pseudo-compiler, --help-sudo.
• git push origin main does not match ["git", "push", "-f", "origin", "main"] (different argv order / extra flags) — the most important case to block slips through.
• curl | bash cannot match anything because subprocess never sees a pipe when shell=False.

The PR description calls out §15.8 will harden this, and the in-file comment is honest. But the v1 denylist gives the illusion of a guardrail without the substance — consider either (a) matching on argv[0] + flag intersection, or (b) making the denylist empty in v1 and landing §15.8 before any agent wires this in. Discuss with maintainer — happy to defer if §15.8 is near.

Recursive tree walks don't prune heavy directories (src/gaia/coder/tools/file.py:597-607, src/gaia/coder/tools/search.py:692)

search_code / find_symbol / list_files all walk .git/, .venv/, node_modules/, __pycache__/. On this repo alone .git/ has ~35k objects and node_modules/ in src/gaia/apps/webui/ has ~300 MB. A single find_symbol("foo") call will read every one of those files before returning. This is a perf regression and a latency footgun for the LLM loop.

Cheapest fix is a default skip-set in _iter_text_files:

_DEFAULT_SKIP_DIRS = frozenset({
    ".git", ".hg", ".svn", ".venv", "venv", "__pycache__",
    "node_modules", ".mypy_cache", ".pytest_cache", ".ruff_cache",
    "dist", "build", ".next",
})


def _iter_text_files(base: Path, glob_pattern: Optional[str]):
    """Yield files under ``base`` that match ``glob_pattern`` (or everything)."""
    if base.is_file():
        if glob_pattern is None or fnmatch.fnmatch(base.name, glob_pattern):
            yield base
        return
    for root, dirs, files in os.walk(base):
        dirs[:] = [d for d in dirs if d not in _DEFAULT_SKIP_DIRS]
        for name in files:
            if glob_pattern is not None and not fnmatch.fnmatch(name, glob_pattern):
                continue
            yield Path(root) / name

Apply the same _DEFAULT_SKIP_DIRS filter inside find_symbol's rglob loop (search.py:692) — or better, route find_symbol through _iter_text_files with glob="*.py".

🟢 Minor

Dead background processes are never GC'd from the registry (src/gaia/coder/tools/cli.py:341-357)

After a background command exits naturally (not via stop_process), its entry stays in _PROCESS_REGISTRY forever — list_processes keeps returning it with running=False. A long-running agent accumulates zombie-like entries. Cheap fix: prune entries where proc.poll() is not None inside list_processes.

SearchToolsMixin.register_search_tools() silently double-registers file tools (src/gaia/coder/tools/search.py:661)

SearchToolsMixin inherits FileToolsMixin and calls self.register_file_tools() inside register_search_tools(). A class that composes both mixins and calls both registration methods (a natural pattern given the KNOWN_TOOLS convention in src/gaia/agents/registry.py) will re-register every file tool. Base @tool decorator overwrites silently, so the bug is invisible until someone monkey-patches. Either document that callers must pick one, or add an idempotency guard.

Timing-dependent test (tests/coder/test_mixins.py:1177)

time.sleep(0.8)
logs = get_process_logs(pid, tail_lines=10)
assert "line" in logs

0.8s is fine on a dev laptop; on a loaded CI runner it will occasionally miss. Prefer polling with a short timeout:

        deadline = time.monotonic() + 5.0
        logs = ""
        while time.monotonic() < deadline:
            logs = get_process_logs(pid, tail_lines=10)
            if "line" in logs:
                break
            time.sleep(0.05)
        assert "line" in logs

search_code reads full file before decode-fail (src/gaia/coder/tools/file.py:546-548)

file_path.read_text(encoding="utf-8") loads the whole blob into memory, then UnicodeDecodeError fires on the first bad byte. A 500 MB vendored binary still gets fully read. A cheap size gate (skip files >1 MB for regex search) would harden this and pairs naturally with the skip-dir fix above.

PR description says draft — do not merge until the scaffold PR lands and wiring is verified end-to-end

It was merged anyway. Not a code issue — but worth flagging to @kovtcharov-amd that merge gates didn't honor the stated intent, and that "wiring verified end-to-end" is precisely the step that would have caught the Critical issue above.

Strengths

• Test coverage is the right shape. Happy / failure / registration triad per tool, with tmp_path isolation and a registry_snapshot fixture that prevents test cross-contamination. test_edit_file_non_unique_raises, test_run_cli_command_denylist_sudo, and test_find_symbol_unsupported_language show care about edge cases.
• Pipe-buffer deadlock avoidance is correct. _spawn_reader draining stdout/stderr on dedicated daemon threads is the standard fix for long-running background processes that produce stdio; easy to get wrong and this PR gets it right.
• edit_file semantics mirror Claude Code Edit. Exact-match + uniqueness enforcement + opt-in replace_all is exactly what the calling LLM expects, and the ValueError messages name the problem in a way an agent can recover from (per CLAUDE.md actionable-errors rule).

Verdict

Approve with follow-up required — the PR itself is good work, but the rebase dropped the __init__.py exports from #819, which breaks tests/coder/test_skeleton.py. That needs a fix-forward PR within a day, along with the private-registry coupling in search.py and the except Exception: pass in cli.py:208. The rest (denylist hardening, skip-dirs, registry GC) can ride §15.8 or a dedicated polish pass.