Add apply_patch tool for multi-file edits in one call - #18
Merged
Conversation
3 tasks
yogthos
force-pushed
the
feature/plan-tools
branch
from
May 19, 2026 16:47
2dcde8b to
58ec4e4
Compare
yogthos
force-pushed
the
feature/apply-patch
branch
from
May 19, 2026 16:48
a761575 to
081e28a
Compare
added 3 commits
May 19, 2026 13:34
Supports create, update (exact text match), delete, and rename operations. Executes in order, stops on first failure, returns summary per operation. Permission checked per file path. Tests cover all four operation types.
- Fix misleading 'atomically' in description — operations execute in order, prior ops remain applied on failure - Add permission check for rename new_path (previously only checked source) - Add 1MB limit on create content to prevent runaway file writes
yogthos
force-pushed
the
feature/apply-patch
branch
from
May 19, 2026 17:34
081e28a to
2fde31b
Compare
yogthos
added a commit
that referenced
this pull request
May 21, 2026
Track F-MEDIUM #18 from ROADMAP.md. ## Problem `is_external_path` (`checker.rs:334-341`) returned `false` for any non-absolute path. In `Accept` mode that's a hole: a relative path with `..` traversal could escape the working directory without triggering the external-path branch: cwd = /home/user/project agent calls read "../../etc/passwd" → is_external_path("../../etc/passwd") = false (not absolute) → Accept mode auto-allows → file leaked to LLM ## Fix `is_external_path` now resolves relative paths via the same `resolve_absolute` used in `check_path`, which canonicalizes when possible (F7) and falls back to lexical parent-canonicalize otherwise. After resolution, the starts_with comparison runs against BOTH the literal `working_dir` and its canonical form — covers macOS's `/tmp → /private/tmp` and similar symlinked roots that would otherwise cause a canonical resolved path to not share a prefix with the literal cwd. If resolve_absolute returns a still-relative path (working_dir is bogus or both canonicalize attempts failed), treat as not- external — rules fall through to the default action, same as the pre-F18 lenient behavior. ## Tests Existing test `relative_path_is_not_external` still passes — in-tree relative paths are unaffected. One new test in `tests/checker_tests.rs`: - `relative_path_escaping_cwd_is_external`: builds a deep tempdir layout `base/a/b/c` (cwd) and `base/escaped/file.rs`. Asserts: - In-tree `local.rs` auto-allows in Accept (regression guard). - `../../../escaped/file.rs` (which canonicalizes to `base/escaped/file.rs`) does NOT auto-allow; surfaces as Ask because it's now correctly classified external. 679 pass (was 678). All build profiles clean. Co-authored-by: Yogthos <yogthos@gmail.com>
yogthos
added a commit
that referenced
this pull request
May 21, 2026
Follow-up to PR #111. Tier-2 items from the 23-bug audit batch: docs corrections and two small correctness/UX fixes. ## Docs - **#12 temperature** — CONFIG.md claimed "parsed but not currently applied". Actually applied since PR #105 with a clamp warning. Rewrote the cell. - **#13 --api-key** — flag existed but neither README nor CONFIG.md mentioned it. Added a Quick-start example noting the process-list visibility caveat. - **#14 acp_host/acp_port** — CONFIG.md documented both keys but the CLI flags were intentionally removed (stdio-only transport). Removed both from the keys table + ACP section. - **#6 tools** — `Config::tools` (per-tool enable map) was fully wired in code but undocumented. Added a row to the keys table covering `tools.websearch` and `tools.webfetch`. - **#21 find_callers** — README claimed "word-boundary regex" but the impl uses the tree-sitter symbol index. Updated to reflect actual behavior; the user-visible word-boundary semantics are preserved. ## Code - **#16 semantic index skip_dir** — `SymbolIndex::find_callers` filter had its own hardcoded `matches!(name, "node_modules" | "target" | ".git" | "__pycache__")` while the rest of the codebase uses `agent::tools::is_skip_dir`. Switched to the shared helper so future additions stay in lockstep. - **#18 context::load_file** — silently swallowed `read_to_string` errors via `.ok()`. A permission-denied AGENTS.md looked identical to a missing file. Now emits a stderr warning naming the path + reason; still returns None so callers' behavior is unchanged. 725 plugin / 599 default pass. All build profiles clean. ## Remaining audit items (deferred to feature work) - **#8 LSP no crash restart**: needs broken-pipe IO error handling + exponential backoff. Touches manager state machine. - **#10 task tool fire-and-forget**: needs timeout + cleanup coordination via JoinHandle tracking. - **#25 MCP no reconnection**: similar architectural concern to #8. - **#27 LSP didClose**: client lifecycle hook missing. - **#29 token estimation len/4**: needs per-provider usage extraction (Phase 6 work). - **#5 MCP shutdown**: rmcp Drop semantics need verification. - **#38/39/40 semantic test gaps**: get_symbol_body untested, list_symbols kind_filter untested, find_definition test vacuous. Sat down to add but each requires a fixture build. Together with PR #111 (10 code fixes), 17 of the 23 verified items are now shipped. Remaining 6 are architectural or test-infrastructure work better tackled as discrete PRs. Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
Track F-MEDIUM dirge-code#18 from ROADMAP.md. ## Problem `is_external_path` (`checker.rs:334-341`) returned `false` for any non-absolute path. In `Accept` mode that's a hole: a relative path with `..` traversal could escape the working directory without triggering the external-path branch: cwd = /home/user/project agent calls read "../../etc/passwd" → is_external_path("../../etc/passwd") = false (not absolute) → Accept mode auto-allows → file leaked to LLM ## Fix `is_external_path` now resolves relative paths via the same `resolve_absolute` used in `check_path`, which canonicalizes when possible (F7) and falls back to lexical parent-canonicalize otherwise. After resolution, the starts_with comparison runs against BOTH the literal `working_dir` and its canonical form — covers macOS's `/tmp → /private/tmp` and similar symlinked roots that would otherwise cause a canonical resolved path to not share a prefix with the literal cwd. If resolve_absolute returns a still-relative path (working_dir is bogus or both canonicalize attempts failed), treat as not- external — rules fall through to the default action, same as the pre-F18 lenient behavior. ## Tests Existing test `relative_path_is_not_external` still passes — in-tree relative paths are unaffected. One new test in `tests/checker_tests.rs`: - `relative_path_escaping_cwd_is_external`: builds a deep tempdir layout `base/a/b/c` (cwd) and `base/escaped/file.rs`. Asserts: - In-tree `local.rs` auto-allows in Accept (regression guard). - `../../../escaped/file.rs` (which canonicalizes to `base/escaped/file.rs`) does NOT auto-allow; surfaces as Ask because it's now correctly classified external. 679 pass (was 678). All build profiles clean. Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…de#112) Follow-up to PR dirge-code#111. Tier-2 items from the 23-bug audit batch: docs corrections and two small correctness/UX fixes. ## Docs - **dirge-code#12 temperature** — CONFIG.md claimed "parsed but not currently applied". Actually applied since PR dirge-code#105 with a clamp warning. Rewrote the cell. - **dirge-code#13 --api-key** — flag existed but neither README nor CONFIG.md mentioned it. Added a Quick-start example noting the process-list visibility caveat. - **dirge-code#14 acp_host/acp_port** — CONFIG.md documented both keys but the CLI flags were intentionally removed (stdio-only transport). Removed both from the keys table + ACP section. - **dirge-code#6 tools** — `Config::tools` (per-tool enable map) was fully wired in code but undocumented. Added a row to the keys table covering `tools.websearch` and `tools.webfetch`. - **dirge-code#21 find_callers** — README claimed "word-boundary regex" but the impl uses the tree-sitter symbol index. Updated to reflect actual behavior; the user-visible word-boundary semantics are preserved. ## Code - **dirge-code#16 semantic index skip_dir** — `SymbolIndex::find_callers` filter had its own hardcoded `matches!(name, "node_modules" | "target" | ".git" | "__pycache__")` while the rest of the codebase uses `agent::tools::is_skip_dir`. Switched to the shared helper so future additions stay in lockstep. - **dirge-code#18 context::load_file** — silently swallowed `read_to_string` errors via `.ok()`. A permission-denied AGENTS.md looked identical to a missing file. Now emits a stderr warning naming the path + reason; still returns None so callers' behavior is unchanged. 725 plugin / 599 default pass. All build profiles clean. ## Remaining audit items (deferred to feature work) - **dirge-code#8 LSP no crash restart**: needs broken-pipe IO error handling + exponential backoff. Touches manager state machine. - **dirge-code#10 task tool fire-and-forget**: needs timeout + cleanup coordination via JoinHandle tracking. - **dirge-code#25 MCP no reconnection**: similar architectural concern to dirge-code#8. - **dirge-code#27 LSP didClose**: client lifecycle hook missing. - **dirge-code#29 token estimation len/4**: needs per-provider usage extraction (Phase 6 work). - **dirge-code#5 MCP shutdown**: rmcp Drop semantics need verification. - **dirge-code#38/39/40 semantic test gaps**: get_symbol_body untested, list_symbols kind_filter untested, find_definition test vacuous. Sat down to add but each requires a fixture build. Together with PR dirge-code#111 (10 code fixes), 17 of the 23 verified items are now shipped. Remaining 6 are architectural or test-infrastructure work better tackled as discrete PRs. Co-authored-by: Yogthos <yogthos@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apply_patchtool: executes an ordered list ofcreate | update | delete | renameoperations on the filesystem. Operations run sequentially and stop on the first failure — prior ops remain applied (description is explicit about this; no transactional rollback).Stacked on #17.
Test plan
cargo build