Repository navigation
apply_patch: take a top-level path as the default for edits that omit their own - #55
Conversation
…hat omit their own
"One file, several edits" is a natural call shape, and a model writes it with the path once
at the top level: `{"path": "a.c", "edits": [{old_string, new_string}, …]}`. The schema
required `path` on every edit, so each such call was rejected before the tool ran
("arguments.edits[0].path is required"). In one run 4 of 10 apply_patch calls had that
shape; the fourth rejection tripped the no-progress breaker and the turn ended with 26 of
30 minutes unused and the deliverable unwritten. An earlier run lost a task at 51/60 tests
the same way.
The top-level `path` is now the default for an edit without one; an edit's own path still
wins; a call with neither names both ways to give one. The schema declares the top-level
property and drops `path` from the per-edit required list.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 7a75657d68dbd43a76c955f1c60ad9f2cdecb8e2)
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Resolved this pass
Before mergeNone. How this fits togetherflowchart LR
n0["...eates_a_new_file_from_an_empty_old_string<br/>changed"]:::changed
n1["test_security"]:::impacted
n2["write"]:::impacted
n3["one_edit"]:::impacted
n4["...s_the_file_state_guard_and_records_writes"]:::impacted
n5["execute_in_context"]:::impacted
n6["apply_patch_enforces_autonomy_and_budgets"]:::impacted
n0 -->|calls| n1
n0 -->|tests| n1
n4 -->|calls| n1
n4 -->|tests| n1
n4 -->|calls| n2
n4 -->|tests| n2
n4 -->|calls| n3
n4 -->|tests| n3
n5 -->|calls| n2
n6 -->|calls| n1
n6 -->|tests| n1
n6 -->|calls| n2
n6 -->|tests| n2
n6 -->|calls| n3
n6 -->|tests| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Warning Review limit reached
This review includes 2 billable files and costs up to $0.50. Or wait 50 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesApply Patch Path Defaults
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change lets apply_patch accept a top-level path as the default for edits that omit their own. Edit-level paths still take precedence, and requests with no path are rejected. No merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new request shape uses the same path checks and write pipeline as explicit edit paths. No new bypass was observed, but production enforcement of workspace isolation and approval requirements could not be verified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1 unsupported.)
A rabbit taps a patch in place, Comment |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0043 · 71,663 in / 4,478 out · 7,094 cached (10%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0019 · 26,753 in / 2,317 out · 576 cached (2%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0023 · 28,775 in / 772 out · 6,326 cached (22%) · gpt-5.6-luna
tests: $0.0000 · 6,050 in / 154 out · 64 cached (1%) · glm-5.3-flash
description: $0.0000 · 5,628 in / 188 out · 0 cached (0%) · glm-5.3-flash
An edit whose `path` was present but not a string previously fell through to the top-level default path, silently patching the wrong file. Such edits now fail with a clear error, and a test covers the malformed case. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Wrap the read_to_string assertions in the malformed-path and per-edit-path tests so they fit the formatter's line width. No test behaviour changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The malformed edit path test now expects the call to return an error instead of a result flagged as an error, matching the updated signature and asserting on the error message directly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The apply_patch tool now accepts an optional top-level path that serves as the default file for edits omitting their own, so callers editing a single file no longer need to repeat the path on every edit. The per-edit path is no longer required and its description documents the fallback. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0039 · 62,427 in / 4,296 out · 8,434 cached (14%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0024 · 22,920 in / 988 out · 2,035 cached (9%) · gpt-5.6-luna
security: $0.0012 · 11,348 in / 609 out · 1,791 cached (16%) · gpt-5.6-luna
tests: $0.0001 · 15,205 in / 880 out · 3,072 cached (20%) · glm-5.3-flash
description: $0.0001 · 7,086 in / 635 out · 1,408 cached (20%) · glm-5.3-flash
A non-string top-level `path` was silently treated as absent, so edits fell back to their own paths instead of failing. It now returns an error, with a test covering the malformed input. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat the assertions in the malformed top-level path test to satisfy rustfmt line width limits. No behaviour change. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
"One file, several edits" is a natural call shape, and a model writes it with the path once
at the top level:
{"path": "a.c", "edits": [{old_string, new_string}, …]}. The schemarequired
pathon every edit, so each such call was rejected before the tool ran("arguments.edits[0].path is required"). In one run 4 of 10 apply_patch calls had that
shape; the fourth rejection tripped the no-progress breaker and the turn ended with 26 of
30 minutes unused and the deliverable unwritten. An earlier run lost a task at 51/60 tests
the same way.
The top-level
pathis now the default for an edit without one; an edit's own path stillwins; a call with neither names both ways to give one. The schema declares the top-level
property and drops
pathfrom the per-edit required list.(cherry picked from commit 7a75657d68dbd43a76c955f1c60ad9f2cdecb8e2)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
apply_patchnow supports a default path for edits that don’t specify their own. An edit-specific path takes precedence.Bug Fixes