From 2369592473c64e6ea56eff4590e8343b641108a3 Mon Sep 17 00:00:00 2001 From: worldmozara Date: Wed, 21 Oct 2026 20:23:54 +0800 Subject: [PATCH] core/tools+cli/display: name the trash in the delete confirm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The box asked "delete path 'x'", which reads as final even now that it is not. It now says "move path 'x' to the system trash" — and "delete path 'x' permanently" when the path really is destroyed: an explicit `permanent=true`, a host that turned `[trash] enabled` off, or a scratch path under the OS temporary directory. The classifier resolves the path to answer that, and falls back to the trash wording when path policy will refuse the call anyway. The pretty line follows: a delete that landed in the trash prints `(to trash)` instead of `(done)`, so a session's transcript still shows which deletes could be undone. --- src/plyngent/cli/display.py | 2 ++ src/plyngent/tools/danger.py | 21 +++++++++++++++++++-- tests/test_cli/test_display.py | 6 ++++++ tests/test_tools/test_danger.py | 20 +++++++++++++++++++- 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/plyngent/cli/display.py b/src/plyngent/cli/display.py index 305bd34..22258fe 100644 --- a/src/plyngent/cli/display.py +++ b/src/plyngent/cli/display.py @@ -550,6 +550,8 @@ def _mutator_detail(name: str, result: str) -> str: if match: chars, _path = match.groups() return _pretty_segments((f"({chars} chars)", None)) + if name == "delete_path" and "system trash" in result: + return _pretty_segments(("(to trash)", "green")) return _pretty_segments(("(done)", "green")) diff --git a/src/plyngent/tools/danger.py b/src/plyngent/tools/danger.py index 293538f..69a65dd 100644 --- a/src/plyngent/tools/danger.py +++ b/src/plyngent/tools/danger.py @@ -6,7 +6,8 @@ from typing import TYPE_CHECKING, cast from plyngent.agent.tools import DangerReason from plyngent.tools.command_scan import dash_c_code, escalators_in_chain, is_interpreter, unwrap_command -from plyngent.tools.workspace import AccessMode, WorkspaceError, resolve_path +from plyngent.tools.trash import under_system_temp +from plyngent.tools.workspace import AccessMode, WorkspaceError, get_trash_enabled, resolve_path if TYPE_CHECKING: from collections.abc import Mapping, Sequence @@ -132,7 +133,23 @@ def _delete_path_reason(args: Mapping[str, object]) -> str | None: path = args.get("path") recursive = bool(args.get("recursive", False)) extra = " recursively" if recursive else "" - return f"delete path {path!r}{extra}" + if _delete_is_final(args): + return f"delete path {path!r}{extra} permanently" + return f"move path {path!r}{extra} to the system trash" + + +def _delete_is_final(args: Mapping[str, object]) -> bool: + """True when the delete destroys the path instead of trashing it.""" + if bool(args.get("permanent", False)): + return True + try: + if not get_trash_enabled(): + return True + target = resolve_path(str(args.get("path", "")), required=AccessMode.WRITE) + except WorkspaceError: + # Path policy will refuse the call; word the confirm as the default. + return False + return under_system_temp(target) def _run_argv_reason(args: Mapping[str, object]) -> DangerReason | None: diff --git a/tests/test_cli/test_display.py b/tests/test_cli/test_display.py index bcfd640..5105980 100644 --- a/tests/test_cli/test_display.py +++ b/tests/test_cli/test_display.py @@ -657,6 +657,12 @@ async def test_pretty_mutators(capsys: pytest.CaptureFixture[str]) -> None: ), ("move_path", '{"src": "src", "dst": "dst"}', "moved directory src -> dst", "* Move 'src' → 'dst' (done)"), ("delete_path", '{"path": "tmp/x.txt"}', "deleted file tmp/x.txt", "* Delete 'tmp/x.txt' (done)"), + ( + "delete_path", + '{"path": "tmp/x.txt"}', + "deleted file tmp/x.txt (moved to system trash)", + "* Delete 'tmp/x.txt' (to trash)", + ), ] for name, args, content, expected in cases: await render_events(_aiter([_pretty_call(name, args), _result(content)])) diff --git a/tests/test_tools/test_danger.py b/tests/test_tools/test_danger.py index fdfa7af..851516b 100644 --- a/tests/test_tools/test_danger.py +++ b/tests/test_tools/test_danger.py @@ -4,6 +4,7 @@ from pathlib import Path from plyngent.agent import DangerReason from plyngent.tools.danger import classify_danger +from plyngent.tools.workspace import set_trash_enabled def _reason(name: str, args: dict[str, object]) -> str | None: @@ -22,11 +23,28 @@ def _danger(name: str, args: dict[str, object]) -> DangerReason: def test_classify_delete_and_move() -> None: - assert _reason("delete_path", {"path": "a.txt"}) == "delete path 'a.txt'" + # No bound workspace here, so the confirm falls back to the default wording. + assert _reason("delete_path", {"path": "a.txt"}) == "move path 'a.txt' to the system trash" assert "recursively" in (_reason("delete_path", {"path": "d", "recursive": True}) or "") assert "move" in (_reason("move_path", {"src": "a", "dst": "b"}) or "") +def test_classify_permanent_delete() -> None: + assert _reason("delete_path", {"path": "a.txt", "permanent": True}) == "delete path 'a.txt' permanently" + + +def test_a_scratch_path_is_announced_as_permanent(workspace: object) -> None: + """The confirm says what will happen: the temp workspace is never trashed.""" + del workspace + assert _reason("delete_path", {"path": "a.txt"}) == "delete path 'a.txt' permanently" + + +def test_the_host_switch_is_announced_as_permanent(workspace: object) -> None: + del workspace + set_trash_enabled(enabled=False) + assert _reason("delete_path", {"path": "a.txt"}) == "delete path 'a.txt' permanently" + + def test_classify_copy() -> None: # Without overwrite flag, copy is not soft-confirmed. assert _reason("copy_path", {"src": "a", "dst": "b"}) is None