diff --git a/Autotests/run_mandatory b/Autotests/run_mandatory index 8aea54a2..8c76355a 100644 --- a/Autotests/run_mandatory +++ b/Autotests/run_mandatory @@ -38,5 +38,6 @@ test_prompt_grounding.py test_websearch_smoke.py test_wschat.py unit/test_fileio_verified_writes.py +unit/test_fileio_verified_deletes.py unit/test_helper_parsing.py unit/test_openclaw_unit.py diff --git a/Autotests/unit/test_fileio_verified_deletes.py b/Autotests/unit/test_fileio_verified_deletes.py new file mode 100644 index 00000000..4b4af02e --- /dev/null +++ b/Autotests/unit/test_fileio_verified_deletes.py @@ -0,0 +1,83 @@ +"""In-process unit tests for src/fileio.py (verified file deletes). + +The delete-file skill returns a result string built from a read-back of the +filesystem after the operation (does the path still exist?) -- ground truth +the agent relays instead of a static success atom it could confabulate +around. Failures return explicit DELETE-FAILED strings instead of raising, +matching the WRITE-VERIFIED / WRITE-FAILED contract in +test_fileio_verified_writes.py. + +No container, no network, no token -- same pattern as +mock_websocket/test_wschat_unit.py: the module is loaded by file path. +""" +import importlib.util +import logging +import os +import sys + +import pytest + +_REPO_ROOT = os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..")) +_FILEIO_PATH = os.path.join(_REPO_ROOT, "src", "fileio.py") + +if _REPO_ROOT not in sys.path: + sys.path.insert(0, _REPO_ROOT) + + +def _load_fileio(): + spec = importlib.util.spec_from_file_location("fileio_under_test", _FILEIO_PATH) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture +def fileio(): + return _load_fileio() + + +def test_delete_removes_existing_file_and_verifies(fileio, tmp_path, caplog): + caplog.set_level(logging.INFO) + target = tmp_path / "out.txt" + target.write_bytes(b"hello world") + + result = fileio.delete_file(str(target)) + + assert not target.exists() + assert result == f"DELETE-VERIFIED file={target}" + assert "[FILE_IO] delete ok" in caplog.text + + +def test_delete_missing_file_fails_without_raising(fileio, tmp_path, caplog): + caplog.set_level(logging.INFO) + target = tmp_path / "absent.txt" + + result = fileio.delete_file(str(target)) + + assert result == f"DELETE-FAILED file={target}: file does not exist" + assert "[FILE_IO] delete failed" in caplog.text + + +def test_delete_directory_fails_without_raising(fileio, tmp_path): + target = tmp_path / "a_directory" + target.mkdir() + + result = fileio.delete_file(str(target)) + + assert target.exists() # directory must be left untouched + assert result == f"DELETE-FAILED file={target}: path is a directory" + + +def test_delete_result_path_matches_input_exactly(fileio, tmp_path): + # regression guard: result string must echo the path as given, not a + # resolved/normalized variant, so the agent can match it back to its request. + sub = tmp_path / "sub" + sub.mkdir() + target = tmp_path / "sub" / ".." / "out.txt" + real = tmp_path / "out.txt" + real.write_bytes(b"x") + + result = fileio.delete_file(str(target)) + + assert not real.exists() + assert result == f"DELETE-VERIFIED file={target}" diff --git a/src/fileio.py b/src/fileio.py index 7ce91a4a..051b2b59 100644 --- a/src/fileio.py +++ b/src/fileio.py @@ -94,3 +94,23 @@ def write_file_b64(path, content_b64): logger.error(f"[FILE_IO] write failed file={path} err=invalid base64") return f"WRITE-FAILED file={path}: invalid base64 ({e})" return _write(path, data, append=False) + + +def delete_file(path): + path = str(path) + if not os.path.lexists(path): + logger.error(f"[FILE_IO] delete failed file={path} err=file does not exist") + return f"DELETE-FAILED file={path}: file does not exist" + if os.path.isdir(path): + logger.error(f"[FILE_IO] delete failed file={path} err=path is a directory") + return f"DELETE-FAILED file={path}: path is a directory" + try: + os.remove(path) + except Exception as e: # the skill must return a result, never raise into the loop + logger.error(f"[FILE_IO] delete failed file={path} err={e}") + return f"DELETE-FAILED file={path}: {e}" + if os.path.exists(path): # race: something recreated it between remove() and here + logger.error(f"[FILE_IO] delete verification failed file={path} still exists after remove") + return f"DELETE-FAILED file={path}: file still exists after removal" + logger.info(f"[FILE_IO] delete ok file={path}") + return f"DELETE-VERIFIED file={path}" \ No newline at end of file diff --git a/src/helper.py b/src/helper.py index 7f71c3e6..a28ad313 100644 --- a/src/helper.py +++ b/src/helper.py @@ -30,7 +30,8 @@ "websearch", "write-file", "get-io-policy", - "write-file-b64" + "write-file-b64", + "delete-file", } LLM_COMMANDS = set(STATIC_LLM_COMMANDS) TWO_ARG_COMMANDS = { diff --git a/src/skills.metta b/src/skills.metta index bca8c9ad..4eb6b997 100644 --- a/src/skills.metta +++ b/src/skills.metta @@ -18,6 +18,7 @@ "- Write base64-encoded content to file as a single line, prefer it when the content contains quotes, backslashes or multiple lines: write-file-b64 filename base64string" "- Append line to existing file, result read back from disk like write-file: append-file filename string" "- Get a list of allowed base paths for reading, writing, and updating files: get-io-policy" + "- Delete a file, the result is read back and verified afterward (DELETE-VERIFIED or DELETE-FAILED with reason) - relay it, never claim a deletion succeeded without it: delete-file filename" ;COMMUNICATION CHANNELS: "- Send message to user: send string" "- Search the web: websearch string" @@ -119,6 +120,9 @@ (= (append-file $file $str) (py-call (fileio.append_file $file $str))) +(= (delete-file $path) + (py-call (fileio.delete_file $path))) + ; Prolog-backed append used by the harness itself (memory.metta appendToHistory): ; it receives a (library ...) file-spec term that only the Prolog open/4 path ; resolves. Body is the previous append-file minus its exists_file guard: