Memo: Remove git_apply_patch from Chukwa and keep the Git tool surface strictly read-only

Purpose

This memo proposes removing git_apply_patch entirely from Chukwa’s MCP tool surface and codebase.

The proposal is based on a direct review of the current codebase, especially:

• Containerfile
• k8s/chukwa.yaml
• src/bin/chukwa-serve.rs
• src/mcp.rs

The conclusion is straightforward:

git_apply_patch is a write-capable tool operating against Chukwa’s runtime repo copy at /app/repo, not against a durable workstation checkout or source-of-truth repository. Because of that, it is misleading and operationally weak. It should be removed rather than fixed or expanded.

What the codebase currently does

1. Chukwa ships a repo copy inside the runtime image

The current Containerfile explicitly copies a repo tree into /app/repo in the runtime image.

The runtime stage:

• installs git
• copies the server binary into the image
• copies selected repo content into /app/repo

The copied content includes:

• Cargo.toml
• src
• tests
• docs
• Containerfile
• k8s
• .git

This is not a live workstation mount. It is an image-bundled repo copy.

2. Kubernetes config points the server at that runtime copy

k8s/chukwa.yaml sets:

• CHUKWA_REPO_DIR=/app/repo

So in the deployed environment, the repo-aware tools are intentionally aimed at the runtime copy.

3. Server startup anchors the repo tools to CHUKWA_REPO_DIR

In src/bin/chukwa-serve.rs, startup reads CHUKWA_REPO_DIR, defaulting to /app/repo, and constructs a RepoRoot from it.

That means the repo root is chosen at process startup and then used by the MCP tool layer.

The startup comments explicitly describe this as the anchor for the codebase navigator, and they note that /app/repo is the location the Containerfile copies source to.

4. git_apply_patch is a first-class MCP tool today

In src/mcp.rs, tool_manifest() currently exposes git_apply_patch as a tool with:

• required patch
• optional dry_run
• optional expected_head
• optional require_clean_worktree
• optional strip
• optional whitespace
• optional allow_binary
• optional max_output_bytes

Its manifest annotation is:

• readOnlyHint = false
• destructiveHint = false

So it is explicitly modeled as a write-capable, non-read-only tool.

5. Tool dispatch routes directly into a write handler

In src/mcp.rs, tools_call() dispatches:

• git_show_commit
• git_log
• git_file_history
• git_diff
• git_apply_patch

and the git_apply_patch arm calls:

• handle_git_apply_patch(&args, env)

That means removal is mechanically simple: the tool is centralized in the manifest and the dispatch table.

6. The implementation really does mutate the anchored repo root

In src/mcp.rs, handle_git_apply_patch begins by resolving the repo root from the environment:

• let repo = repo_or_err(env)?;
• let repo_dir = repo.root().to_string_lossy().to_string();

From there it:

• parses patch paths
• validates path safety
• computes a patch SHA256
• checks HEAD
• optionally checks for a dirty worktree
• writes the patch to a temp file
• runs git apply --numstat
• runs git apply --check
• and, when not dry_run, runs actual git apply

It also checks HEAD again afterward and reports repo context.

The key point is this:

the handler applies the patch to the repo root that Chukwa is anchored to, which in deployed mode is /app/repo.

7. The handler never commits

The handler modifies the working tree but does not create a commit.

That is not an inference; it is part of the tool’s own contract in the manifest description and visible in the implementation shape:

• it checks HEAD
• applies the patch
• checks HEAD again
• returns result metadata

There is no commit step.

So even in the best case, the tool only edits a worktree. In the deployed case, that worktree is the runtime repo copy.

Why this is a problem

1. The tool writes to the wrong place for real software change

The deployed server is pointed at /app/repo, which is an image-bundled repo copy.

So git_apply_patch may truthfully report:

• patch applied
• files changed
• additions/deletions
• touched paths

while still not changing the durable source tree that operators actually use for:

• commit
• push
• deploy
• long-term maintenance

That makes the tool misleading.

2. It creates a shadow-editing surface

The read-only tools are fine against a repo copy because they are observational.

The moment a write tool is added, the distinction becomes dangerous:

• the tool looks like it changes “the codebase”
• but it only changes the runtime copy
• and it does so without commit semantics
• and without deployment semantics

That is not a useful source-control workflow.

3. Safety features do not solve the core problem

The tool has substantial safety work:

• path sanitization
• strip-aware protected-path checks
• binary patch rejection
• expected_head
• dirty-worktree guard
• dry-run default
• repo-context reporting

Those are all real and thoughtful.

But they solve “how to apply a patch safely inside the anchored repo root.” They do not solve “is this the repo that matters?”

The failure is architectural, not just operational.

4. Keeping it would pressure the system toward a worse security model

The only way to make git_apply_patch truly useful would be to give it access to a durable checkout that matters operationally.

That would push the system toward exposing a real writable repo surface through the MCP server.

That is exactly the kind of security and boundary erosion we do not want.

So the right move is not to empower the tool. It is to delete it.

What should be removed

The code review shows the removal scope is concrete and manageable.

A. Remove the tool manifest entry

In src/mcp.rs, remove the git_apply_patch entry from tool_manifest().

That includes:

• the name
• the description
• the input schema
• the annotations

B. Remove the dispatch arm

In src/mcp.rs, remove the tools_call() match arm:

• "git_apply_patch" => handle_git_apply_patch(&args, env),

Once that is gone, requests for git_apply_patch will naturally fall into the existing unknown-tool path.

C. Remove the handler

In src/mcp.rs, remove:

• handle_git_apply_patch

D. Remove the patch-application helper block

src/mcp.rs currently contains a dedicated helper section for patch application. Based on the current structure, the following helper functions are patch-specific and should be removed if nothing else uses them:

• parse_patch_paths_raw
• split_diff_git_paths
• apply_strip
• check_safe_dest_path
• patch_has_binary
• parse_apply_numstat

These helpers exist to support patch ingestion and safe application. They are not part of the read-only Git inspector surface.

E. Remove apply-specific tests

The tests module in src/mcp.rs contains a dedicated git_apply_patch test region.

The current codebase includes apply-specific tests covering things like:

• dirty-worktree rejection
• expected-head mismatch
• manifest exposure
• protected path / strip behavior
• repo-context reporting for apply/dry-run

Representative test names in the current code include:

• git_apply_expected_head_mismatch
• git_apply_patch_appears_in_tools_list
• git_apply_strip_cannot_bypass_git_protection
• git_apply_patch_repo_context_dry_run_and_apply

That whole test surface should be removed along with the tool.

What should remain

The proposal is to remove only the write-capable patch tool, not the read-only repo tooling.

These handlers should remain:

• handle_git_show_commit
• handle_git_log
• handle_git_file_history
• handle_git_diff

These code-navigation handlers should also remain:

• handle_browse_codebase
• handle_list_code_files
• handle_search_code
• handle_read_code
• handle_outline
• handle_find_definition
• handle_find_references

The supporting shared Git helpers should remain where still needed, including functions like:

• run_git
• git_repo_context
• path validation that is used by read-only Git handlers

The runtime image should also continue shipping Git and .git as long as the read-only Git inspector tools remain part of the product.

What should not be done

This ticket should not become a broader repo-architecture redesign.

It should not:

• mount a workstation checkout
• try to sync /app/repo with a live source checkout
• redesign CHUKWA_REPO_DIR
• remove the read-only Git tools
• remove the code-navigation tools
• leave a deprecated or aliased git_apply_patch
• replace it with a renamed write tool
• add a compatibility tombstone
• introduce a fallback behavior

This should be a clean expunging of a failed experiment.

Policy decision to encode

After this change, the Git tool family should be explicitly read-only.

That is already almost true in practice. The only exception is git_apply_patch.

Removing it would make the repo/Git tool story coherent:

• inspect code
• inspect history
• inspect diffs
• do not mutate repos through the MCP server

Why this is better than trying to “fix” the tool

A smaller patch could improve one symptom, such as noisy dirty-worktree checks.

But that would not solve the real issue.

The real issue is that git_apply_patch mutates an anchored runtime repo copy, not a durable source-of-truth checkout.

So even a more polished git_apply_patch would remain conceptually wrong for this system.

The codebase already gives us a clean boundary:

• read-only repo introspection in the MCP server
• real code change, commit, and deployment outside that boundary

We should preserve that boundary.

Acceptance criteria

This work is complete when all of the following are true:

1. git_apply_patch no longer appears in tool_manifest().

2. tools_call() no longer dispatches git_apply_patch.

3. handle_git_apply_patch is removed.

4. The patch-only helper functions are removed.

5. The patch-only tests are removed.

6. Read-only Git tools still work:

   • git_show_commit
   • git_log
   • git_file_history
   • git_diff

7. Read-only code-navigation tools still work.

8. cargo test passes.

9. There is no deprecation alias, fallback, or tombstone for git_apply_patch.

Verification plan

After implementation:

• run cargo test
• confirm tools/list no longer includes git_apply_patch
• confirm requests for git_apply_patch return the ordinary unknown-tool behavior
• confirm the remaining Git inspector tools still function
• confirm the remaining code-navigation tools still function

A useful final grep would be to inspect for any leftover patch-tool references in src/mcp.rs.

Final recommendation

Remove git_apply_patch completely.

The current codebase makes clear that it is a write-capable tool bound to Chukwa’s anchored runtime repo root, which is configured in deployed environments as /app/repo. That makes it unsuitable as a real code-change mechanism and too misleading to keep around.

The cleanest, safest, and most coherent outcome is:

• delete git_apply_patch
• delete its helpers
• delete its tests
• keep the Git and code-navigation tools read-only

That gives Chukwa a clearer boundary and removes a tool that does not deliver meaningful operational value.

Implemented in commit eb1cac1 (deployed, pushed). git_apply_patch is completely excised from the MCP surface and codebase; the git tool family is now strictly read-only.

Live verification against the deployed pod (chukwa-7d5b75ff6d-9btsp)

1. Total MCP tools: 35
   git_apply_patch present: False
   Remaining git_* tools: git_show_commit, git_log, git_file_history, git_diff

2. Calling the removed tool => code=UNKNOWN_TOOL

3. git_log still works: 1 commit
   HEAD: eb1cac1 refactor(mcp): remove git_apply_patch; keep git surface read-only
   repo_context.current_branch: main

4. read_code still works: Cargo.toml line 1 = [package]

Acceptance criteria

• git_apply_patch no longer appears in tool_manifest().
• tools_call() no longer dispatches git_apply_patch.
• handle_git_apply_patch removed.
• Patch-only helper functions removed: parse_patch_paths_raw, split_diff_git_paths, apply_strip, check_safe_dest_path, patch_has_binary, parse_apply_numstat.
• Patch-only tests removed (13 of them: dry-run, apply, invalid patch, unsafe path, dirty worktree, expected_head match/mismatch, empty patch, manifest exposure, strip bypass, strip honoured, strip zero, repo_context apply/dry-run).
• Read-only git tools still work: git_show_commit, git_log, git_file_history, git_diff (verified live).
• Read-only code-navigation tools still work: read_code verified; others untouched.
• cargo test passes: 209 lib + 15 phase0 = 224 tests green.
• No deprecation alias, fallback, or tombstone for git_apply_patch.

What changed, in one diff

$ git diff b04d566..eb1cac1 --stat
 src/mcp.rs | 664 ++-----------------------------------------------------------
 1 file changed, 22 insertions(+), 642 deletions(-)

What stayed

• Read-only git MCP tools and their handlers.
• Code-navigation tools and their handlers.
• Shared git helpers that read-only tools still need: run_git, git_repo_context, validate_git_path, truncate_to_char_boundary, parse_git_log_records.
• The container image still ships git + .git (needed by read-only git tools).
• Test helpers git_repo, git_env, git_head (repo_context tests still build real on-disk repos through them).

New guard test

Added git_apply_patch_not_in_tool_manifest — mirrors the existing get_repo_zip_not_in_tool_manifest. Any future re-introduction of the write tool by name fails at test time.

Policy note

The repo/Git tool story is now coherent — inspect code, inspect history, inspect diffs, do not mutate repos through the MCP server. Real code change happens outside that boundary (workstation -> git push -> ./k8s/deploy.sh).