From f13ea9f316f3a4f6fa24c97bd90707abaf0db47b Mon Sep 17 00:00:00 2001 From: worldmozara Date: Sun, 19 Jul 2026 03:05:01 +0800 Subject: [PATCH] core/tools: soft-confirm file overwrite only, not partial edits --- src/plyngent/tools/danger.py | 41 +++++++++++++++------------------ tests/test_tools/test_danger.py | 24 +++++++++++++++---- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/plyngent/tools/danger.py b/src/plyngent/tools/danger.py index 010c6f4..20e97b4 100644 --- a/src/plyngent/tools/danger.py +++ b/src/plyngent/tools/danger.py @@ -110,36 +110,37 @@ def _shell_or_dash_c_reason(argv: Sequence[str], *, via: str) -> str | None: def _write_file_reason(args: Mapping[str, object]) -> str | None: + """Confirm only when write_file would replace an existing file.""" path = args.get("path") if not isinstance(path, str) or not path: return None try: target = resolve_path(path) except WorkspaceError: - return f"write file {path!r}" - return f"write file {path!r} ({target})" - - -def _edit_replace_reason(args: Mapping[str, object]) -> str | None: - path = args.get("path") - if not isinstance(path, str) or not path: + # Path policy will fail later; no soft-confirm without a resolved target. return None - return f"edit (replace) in {path!r}" - - -def _edit_lineno_reason(args: Mapping[str, object]) -> str | None: - path = args.get("path") - if not isinstance(path, str) or not path: - return None - start = args.get("start_line") - end = args.get("end_line") - return f"edit lines {start}-{end} in {path!r}" + if target.is_file(): + return f"overwrite existing file {path!r} ({target})" + # New file or directory path: no total-overwrite risk for soft-confirm. + return None def _copy_path_reason(args: Mapping[str, object]) -> str | None: + """Confirm copy only when it would replace an existing destination path.""" src = args.get("src") dst = args.get("dst") - return f"copy {src!r} → {dst!r}" + overwrite = bool(args.get("overwrite", False)) + if not overwrite: + return None + if not isinstance(dst, str) or not dst: + return f"copy {src!r} → {dst!r} (overwrite)" + try: + target = resolve_path(dst) + except WorkspaceError: + return None + if target.exists(): + return f"copy {src!r} → overwrite existing {dst!r} ({target})" + return None def _move_path_reason(args: Mapping[str, object]) -> str | None: @@ -222,10 +223,6 @@ def classify_danger(name: str, args: Mapping[str, object]) -> str | None: # noq return _copy_path_reason(args) if name == "write_file": return _write_file_reason(args) - if name == "edit_replace": - return _edit_replace_reason(args) - if name == "edit_lineno": - return _edit_lineno_reason(args) if name == "run_command": return _run_command_reason(args) if name == "run_command_batch": diff --git a/tests/test_tools/test_danger.py b/tests/test_tools/test_danger.py index cc5cf8e..8d7f75c 100644 --- a/tests/test_tools/test_danger.py +++ b/tests/test_tools/test_danger.py @@ -12,18 +12,32 @@ def test_classify_delete_and_move() -> None: def test_classify_copy() -> None: - assert "copy" in (classify_danger("copy_path", {"src": "a", "dst": "b"}) or "") + # Without overwrite flag, copy is not soft-confirmed. + assert classify_danger("copy_path", {"src": "a", "dst": "b"}) is None + assert classify_danger( + "copy_path", {"src": "a", "dst": "b", "overwrite": False} + ) is None -def test_classify_write_and_edits(tmp_path: Path) -> None: +def test_classify_write_overwrite_only(tmp_path: Path) -> None: from plyngent.tools.workspace import clear_workspace_root, set_workspace_root set_workspace_root(tmp_path) try: + # New file: no soft-confirm + assert classify_danger("write_file", {"path": "new.txt"}) is None + # Partial edits: never soft-confirm + assert classify_danger("edit_replace", {"path": "x.txt"}) is None + assert classify_danger("edit_lineno", {"path": "x.txt", "start_line": 1, "end_line": 2}) is None + # Existing file write: confirm total overwrite + (tmp_path / "x.txt").write_text("old", encoding="utf-8") reason = classify_danger("write_file", {"path": "x.txt"}) - assert reason is not None and "write file" in reason - assert "edit" in (classify_danger("edit_replace", {"path": "x.txt"}) or "") - assert "lines" in (classify_danger("edit_lineno", {"path": "x.txt", "start_line": 1, "end_line": 2}) or "") + assert reason is not None and "overwrite" in reason + (tmp_path / "dst.txt").write_text("d", encoding="utf-8") + (tmp_path / "src.txt").write_text("s", encoding="utf-8") + assert classify_danger("copy_path", {"src": "src.txt", "dst": "dst.txt", "overwrite": False}) is None + creason = classify_danger("copy_path", {"src": "src.txt", "dst": "dst.txt", "overwrite": True}) + assert creason is not None and "overwrite" in creason finally: clear_workspace_root()