Skip to content

Commit 680cdcf

Browse files
committed
fix: avoid overwriting new file targets
1 parent 94c8699 commit 680cdcf

5 files changed

Lines changed: 52 additions & 14 deletions

File tree

prompts/file_patch_repair.md

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,14 @@ markdown, prose, or shell commands. Use exactly this top-level shape:
3030
}}
3131
```
3232

33-
Do not use `--- /dev/null` for a target file that already exists; generate an
34-
update diff against the existing file. If the current file snapshot is present
35-
in the failure context, base the hunk context on that snapshot. If the failure
36-
mentions `expected=...` and `actual=...`, the previous hunk context is stale:
37-
use the `actual` line and the current snapshot as the source of truth, then
38-
return a new diff whose context lines exactly match the current file.
33+
Do not use `--- /dev/null` for a target file that already exists. If the
34+
original request asked to create a new file and the chosen filename is already
35+
taken, inspect the sibling directory with `list_dir` and return a create diff
36+
for a clear unused filename in that same directory. Do not update, overwrite, or
37+
rewrite the existing file unless the original request asked to edit/update that
38+
file. If the current file snapshot is present in the failure context and the
39+
user asked to update that existing file, base the hunk context on that snapshot.
40+
If the failure mentions `expected=...` and `actual=...`, the previous hunk
41+
context is stale: use the `actual` line and the current snapshot as the source
42+
of truth, then return a new diff whose context lines exactly match the current
43+
file.

prompts/planner.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,11 @@ shown to the user before any file is changed. Return only this JSON object:
9999
```
100100

101101
For new files, use `--- /dev/null` and `+++ /absolute/or/relative/path` in the
102-
unified diff. If read-only tools show the target file already exists, generate
103-
an update diff with the existing path in both headers; never use `/dev/null` for
104-
an existing target. Do not apply the patch through shell commands; the graph
105-
applies FilePatchPlan after human confirmation. If a generated script needs
106-
executable permissions, use `permission_changes`; do not emit `chmod` as a shell
107-
command.
102+
unified diff. When the user asks to create a new file, first inspect the target
103+
directory; if your preferred filename already exists, choose a clear unused
104+
filename in the same directory instead of updating or overwriting the existing
105+
file. Only generate an update diff with the existing path in both headers when
106+
the user asked to edit/update that existing file. Do not apply the patch through
107+
shell commands; the graph applies FilePatchPlan after human confirmation. If a
108+
generated script needs executable permissions, use `permission_changes`; do not
109+
emit `chmod` as a shell command.

src/linuxagent/graph/file_patch_nodes.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ def _should_repair_patch_safety_failure(state: AgentState, safety: FilePatchSafe
359359
def _is_repairable_patch_error(reasons: str) -> bool:
360360
return (
361361
"unified diff context does not match target file" in reasons
362-
or "target already exists; use an update diff instead of a create diff" in reasons
362+
or "target already exists" in reasons
363363
)
364364

365365

src/linuxagent/plans/file_patch.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,8 @@ def _planned_file_update(patch: _FilePatch) -> _PlannedFileUpdate:
282282
target = _target_path(patch)
283283
if patch.old_path == "/dev/null" and target.exists():
284284
raise FilePatchApplyError(
285-
"target already exists; use an update diff instead of a create diff",
285+
"target already exists; create requests must choose an unused filename, "
286+
"while edit requests must use an update diff",
286287
path=target,
287288
)
288289
old_lines = _read_lines(target)

tests/unit/graph/test_agent_graph.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,36 @@ async def test_graph_repairs_failed_file_patch_and_reconfirms(tmp_path) -> None:
522522
assert "analysis ok" in str(resumed["messages"][-1].content)
523523

524524

525+
async def test_graph_repairs_new_file_name_collision_with_unused_name(tmp_path) -> None:
526+
existing = tmp_path / "disk_info.sh"
527+
alternate = tmp_path / "disk_info_1.sh"
528+
existing.write_text("#!/bin/sh\necho existing\n", encoding="utf-8")
529+
first_plan = file_patch_plan_json(str(existing), "#!/bin/sh\necho disk\n")
530+
repaired_plan = file_patch_plan_json(str(alternate), "#!/bin/sh\necho disk\n")
531+
graph, _provider = _graph(tmp_path, [first_plan, repaired_plan, "analysis ok"])
532+
config = {"configurable": {"thread_id": "new-file-collision"}}
533+
534+
await graph.ainvoke(
535+
initial_state("create a new disk info shell script in tmp", source=CommandSource.USER),
536+
config=config,
537+
)
538+
snapshot = await graph.aget_state(config)
539+
interrupts = snapshot.tasks[0].interrupts
540+
541+
assert interrupts[0].value["type"] == "confirm_file_patch"
542+
assert interrupts[0].value["repair_attempt"] == 1
543+
assert str(alternate) in interrupts[0].value["files_changed"]
544+
assert str(existing) not in interrupts[0].value["files_changed"]
545+
546+
resumed = await graph.ainvoke(
547+
Command(resume={"decision": "yes", "latency_ms": 1}), config=config
548+
)
549+
550+
assert existing.read_text(encoding="utf-8") == "#!/bin/sh\necho existing\n"
551+
assert alternate.read_text(encoding="utf-8") == "#!/bin/sh\necho disk\n"
552+
assert "analysis ok" in str(resumed["messages"][-1].content)
553+
554+
525555
async def test_graph_blocks_file_patch_outside_allow_roots(tmp_path) -> None:
526556
target = tmp_path / "blocked" / "demo.sh"
527557
graph, _provider = _graph(

0 commit comments

Comments
 (0)