Skip to content

apply_patch: take a top-level path as the default for edits that omit their own - #55

Merged
senamakel merged 7 commits into
tinyhumansai:mainfrom
sanil-23:pr/apply-patch-top-level-path
Oct 8, 2026
Merged

senamakel merged 7 commits into
tinyhumansai:mainfrom
sanil-23:pr/apply-patch-top-level-path

Conversation

@sanil-23

@sanil-23 sanil-23 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

"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.

(cherry picked from commit 7a75657d68dbd43a76c955f1c60ad9f2cdecb8e2)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • apply_patch now supports a default path for edits that don’t specify their own. An edit-specific path takes precedence.
  • Bug Fixes

    • Invalid edit paths are rejected instead of silently using the default, and missing paths produce a clear error. Failed requests leave the target unchanged.

…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)
@tinysweeper

tinysweeper Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny 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
Priority: none
Reviewed head: bfd90b45996b
Updated: 1791459319 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 2 Noted findings 0
Documentation 0 Resolved findings 12
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Resolved this pass

  • Reject malformed per-edit paths before applying the default
  • Reject a non-string top-level path
  • Reject a non-string top-level path instead of treating it as missing
  • Reject malformed per-edit paths before applying the default
  • Reject a non-string top-level path
  • Reject a non-string top-level path instead of treating it as missing
  • Reject malformed per-edit paths before applying the default
  • Reject a non-string top-level path
  • Reject a non-string top-level path instead of treating it as missing
  • Reject malformed per-edit paths before applying the default
  • Reject a non-string top-level path
  • Reject a non-string top-level path instead of treating it as missing

Before merge

None.

How this fits together

flowchart 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
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The top-level default path is implemented with explicit validation, per-edit overrides, and malformed-path rejection. The previously reported path-validation issues are fixed, and this change looks safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change safely supports a top-level default path while preserving per-edit overrides and rejecting malformed paths. The earlier concerns are fixed, and the change looks safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The top-level default-path behaviour is implemented and pinned by five tests covering the happy path, per-edit override, malformed per-edit path, malformed top-level path, and no path at all; the earlier findings about rejecting a malformed per-edit path and a non-string top-level path are both fixed with producing tests. Safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change adds a top-level `path` default for apply_patch edits, with the schema and description updated to match, and all three earlier findings are now fixed: a malformed per-edit `path` is rejected rather than silently defaulted (`apply_patch_rejects_malformed_edit_path_instead_of_using_default`), and a non-string top-level `path` is rejected with a clear error (`apply_patch_rejects_a_malformed_top_level_path`). Tests cover the default, the override, and the neither-given error path. The description accurately describes the diff; nothing new to raise. Safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: gpt-5.6-luna, glm-5.3-flash
  • Spend: $0.006571
  • Tokens: 88031 input · 3779 output · 12128 cached · 0 embedding
Head State Pass summary
5e971fc2f09e ready for maintainer review 1 active finding(s), 0 resolved finding(s) (at 1791440205)
d8407ea216a5 ready for maintainer review 1 active finding(s), 4 resolved finding(s) (at 1791458340)
0d6d9faba052 ready for maintainer review 2 active finding(s), 4 resolved finding(s) (at 1791458429)
bfd90b45996b ready for maintainer review 0 active finding(s), 12 resolved finding(s) (at 1791459319)

tinysweeper 0.1.0

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 50 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 71e3e03a-c64f-4d7b-b391-2c997ff63588
📥 Commits

Reviewing files that changed from the base of the PR and between 0d6d9fa and bfd90b4.

📒 Files selected for processing (2)
  • crates/tinytools-std/src/filesystem/apply_patch/mod.rs
  • crates/tinytools-std/src/filesystem/apply_patch/mod_tests.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b3dab694-882a-47f7-8014-f0a4b3956854
📥 Commits

Reviewing files that changed from the base of the PR and between 68163b1 and 0d6d9fa.

📒 Files selected for processing (3)
  • crates/tinytools-std/src/filesystem/apply_patch/mod.rs
  • crates/tinytools-std/src/filesystem/apply_patch/mod_tests.rs
  • crates/tinytools-std/src/filesystem/fixtures/apply_patch.json

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

apply_patch accepts an optional top-level path for edits that omit their own path. An edit-level path takes precedence and must be a string. The schema and tests cover fallback, precedence, invalid paths, and missing paths.

Changes

Apply Patch Path Defaults

Layer / File(s) Summary
Path fallback contract and execution
crates/tinytools-std/src/filesystem/apply_patch/mod.rs, crates/tinytools-std/src/filesystem/fixtures/apply_patch.json, crates/tinytools-std/src/filesystem/apply_patch/mod_tests.rs
The schema documents the optional top-level path and makes per-edit path optional. Execution uses the top-level value when an edit omits its path, while a supplied edit path takes precedence and must be a string. Tests cover fallback, precedence, invalid paths, and missing paths.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to 0d6d9

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 Review

Security architecture risk: 🔵 Low · up to 0d6d9

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The relevant outcome is modification or creation of gate-approved files, with up to 50 edits per request. Any fallback request can express the same selected targets through explicit edit paths, so no additional filesystem authority is demonstrated. Actual reachable roots, tenant isolation, sensitive-file exclusions, and operating-system privileges depend on the unavailable production host policy.

Security Findings and Attack Paths

  • inferred — No introduced bypass is established in the inspected path-selection flow. Model-supplied fallback paths cannot skip the common policy pipeline, and an explicitly malformed edit path is rejected rather than redirected to the default. This does not prove production containment or approval enforcement.

Trust Boundaries and Controls

  • observed — FsGate is the injected host-owned authority boundary. ApplyPatchTool advertises write permission, consults the host's approval requirement, checks mutation permission and budgets, and delegates final path authorization. These delegation points are visible, but their concrete production implementation and caller identity binding remain unverified.

Resilience and Maintainability Implications

  • observed — The existing pipeline attempts canonical-path locking and stale/partial-read checks, but lock acquisition is conditional on successful canonicalization. Recovery uses in-memory snapshots rather than durable transactions. Concurrent creation and process interruption therefore remain important host-level limitations; neither mechanism nor its exposure is shown to be worsened by this PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: using a top-level path as the default for edits that omit their own path.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

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.)

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit taps a patch in place,
With one top path to set the pace.
Each edit may choose its own,
Or use the path that’s softly shown.
The files now rest, their changes sewn.

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread crates/tinytools-std/src/filesystem/apply_patch/mod.rs Outdated
@senamakel senamakel self-assigned this Oct 8, 2026
senamakel and others added 4 commits October 8, 2026 14:15
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>

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread crates/tinytools-std/src/filesystem/apply_patch/mod.rs Outdated
senamakel and others added 2 commits October 8, 2026 14:22
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>
@senamakel
senamakel merged commit 86c37ce into tinyhumansai:main Oct 8, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants