[OMEGA-331] Add delete-file skill with existence check and error handling

[Aryam-21]

Summary

Adds a delete-file skill that safely removes a file, matching the
verified-result contract established by write-file/append-file/
write-file-b64 in #243: no static success atom, no silent failures —
every result is built from a read-back of the filesystem after the
operation.

Implementation

src/fileio.py — new delete_file(path):

• Checks the path exists and isn't a directory before attempting removal
• Performs the delete inside a try/except that never raises into the
  caller — failures return an explicit string, matching write_file's
  contract
• Re-checks the filesystem afterward to confirm the file is actually
  gone (guards against a race where something recreates it mid-call)
• Returns DELETE-VERIFIED file=<path> existed=true on success, or
  DELETE-FAILED file=<path>: <reason> on failure (not found / is a
  directory / OS error / still exists after removal)

src/skills.metta:

• delete-file now calls (py-call (fileio.delete_file $path)),
  consistent with how write-file/append-file route through
  fileio.py rather than raw Prolog predicates
• Updated the skill description in getStaticSkills to reflect the new
  DELETE-VERIFIED/DELETE-FAILED contract

Autotests/unit/test_fileio_verified_deletes.py — new unit tests,

• Successful delete, verified via read-back
• Missing-file failure (no exception raised)
• Directory guard (refuses to delete a directory, leaves it untouched)
• Path-echo regression check (result string matches the path as given,
  not a resolved/normalized variant, so the agent can match it back to
  its request)

Review history

An earlier version used a raw (translatePredicate (exists_file ...))
guard with a static DELETE-FILE-SUCCESS atom. Per review feedback,
this was reworked to:

1. Route through fileio.py with read-back verification instead of a
   static success atom (the agent otherwise can't distinguish an actual
   deletion from a claimed one)
2. Drop the raw exists_file guard, which yields Empty rather than
   False when used directly as an if condition — a pitfall already
   hit once during the [OMEGA-273][FIX] skills: file-write results are read back from disk (add write-file-b64) #243 rebase
3. Add unit tests for both branches

This branch was also rebased/merged against main to pick up the #243
file-I/O rework that landed after this branch was created.

Testing

• python -m pytest Autotests/unit/test_fileio_verified_deletes.py -v
  — 4/4 passing
• Verified end-to-end via a live agent session (Mistral provider):
  write-file created a file, delete-file removed it, confirmed via
  an independent ls check outside the agent
• Confirmed the not-found branch (DELETE-FAILED ... file does not exist) via a real MeTTa-level call through the full run.metta
  bootstrap chain

[MartinEbner]

Nice addition — an explicit not-found branch instead of a silent failure is the right instinct.
A couple of things worth checking before this lands, both from the file-I/O rework in #243, plus a
minor note:

1. write-file / append-file no longer return static success atoms. Since #243 they are
Python-backed (src/fileio.py) and return a result built from a read-back of the file on disk:

WRITE-VERIFIED file=/tmp/x.txt bytes=201 sha256=e02bd6c2bec42055 head='…' tail='…'

The reason was that a static …-SUCCESS atom certifies only that the call did not raise — the
agent cannot tell an actual write from a claimed one, and in practice it relays the success atom
as fact. DELETE-FILE-SUCCESS reintroduces exactly that gap for deletion. Routing delete-file
through src/fileio.py and confirming the file is gone afterwards (e.g.
DELETE-VERIFIED file=… existed=true / an explicit DELETE-FAILED file=…: <reason>) would keep
the whole file-I/O surface on one contract, and you would get the failure branches — permission
denied, path is a directory, races — for free instead of them surfacing as a generic error.

2. The bare exists_file guard has a subtle failure mode. src/utils.metta wraps it:

(= (exists-file $path)
   (empty-to-bool (translatePredicate (exists_file $path))))

That wrapper exists because (translatePredicate (exists_file $path)) yields Empty rather than
False when the file is missing, so using the raw goal directly as an if condition does not
reliably take the else branch. This bit the #243 branch during a rebase: an exists_file guard
left in an append-file clause failed as a goal instead of returning false, and the enclosing
expression died silently rather than returning the intended error. Using the existing
(exists-file $path) helper here is a small change and avoids that class entirely.

3. Minor: the repo now runs unit tests in CI (tests/ host-side, plus Autotests/ entries
wired into Autotests/run_mandatory) — a small test for the two branches would let this be
verified without a live agent session.

Happy to help with the fileio.py side if useful.

[MartinEbner]

That is a thorough rework — the directory case and the post-remove() re-check are both good
catches that were not in the suggestion. Three things left:

1. delete-file needs registering in the command set in src/helper.py, or the skill cannot be
invoked at all once this merges. Heads-up on timing: #301 (reject unknown skill calls) merged a
few hours ago, after this branch's last update, so you will not see this locally until you pull
main — it renamed the set to STATIC_LLM_COMMANDS (with LLM_COMMANDS derived from it) and
made balance_parentheses turn any unregistered command into (Error UNKNOWN_SKILL_CALL …).
Because this PR does not touch helper.py, it still merges cleanly — it would just land a skill
the parser refuses. Against main as it stands now:

>>> balance_parentheses('delete-file /tmp/out.txt')
'((Error UNKNOWN_SKILL_CALL "delete-file /tmp/out.txt"))'

>>> balance_parentheses('delete-file /tmp/out.txt\nsend done')
'((Error UNKNOWN_SKILL_CALL "delete-file /tmp/out.txt") (send "done"))'

>>> balance_parentheses('read-file /tmp/out.txt')      # registered, for contrast
'((read-file "/tmp/out.txt"))'

So every delete-file the model emits would come back as ALERT_FAILED feedback and the file
would never be touched — fileio.delete_file and its tests correct, but unreachable. Adding
"delete-file" to STATIC_LLM_COMMANDS after rebasing fixes it. It does not belong in
TWO_ARG_COMMANDS — that is for commands taking a filename and a payload, and delete-file
takes only the path.

(Pre-#301 the same omission was milder but still wrong: starts_command_line() did not recognise
the line, so a delete-file that was not first in a multi-command emission got appended to the
previous command's block instead of being parsed as its own.)

2. Autotests/unit/test_fileio_verified_deletes.py is not wired into Autotests/run_mandatory.
CI executes only the files listed in that arg-file, so the four new tests will not run in the
pipeline — adding the path there (next to unit/test_fileio_verified_writes.py) is a one-line
change and makes the coverage real.

3. Cosmetic: src/fileio.py now ends without a trailing newline, and delete_file starts
immediately after write_file_b64's return with no blank line — the other top-level functions
in that file are separated by two.

One thing that is my fault rather than yours: I suggested the existed=true field, and looking at
it in place it is a constant — the not-exists case already returns DELETE-FAILED, so the field
never says anything else. Dropping it would lose nothing.

[timur-ashkenov]

@Aryam-21
Please rebase this branch onto the current main. It has diverged, and the changes overlap in src/helper.py and Autotests/run_mandatory, so the PR does not apply cleanly as-is.

[timur-ashkenov]

os.path.exists() returns False for dangling symlinks, so delete-file reports a missing file and leaves the symlink behind. I think its better to use os.path.lexists() here.

[TossSky]

Checked this for OMEGA-331.

• delete-file is in the static command list, so the parser treats it as a command boundary
• unit/test_fileio_verified_deletes.py is registered in run_mandatory and contributes 4 tests
• full mandatory suite in a container: 131 passed
• read-back is real: DELETE-VERIFIED only after os.remove plus a check that the path is gone
• probed against a real filesystem: regular file, missing file, empty and non-empty directory, unwritable parent, symlink, dangling symlink, empty path, path with ..
• under the policy the agent runs with, /PeTTa, /etc and traversal out of memory/ are refused and the files stay on disk

One gap for later: the file still exists after removal branch is the only one without a test, and symlink behaviour (the link goes, the target stays) is untested too.

Verdict: PASS

[Aryam-21]

Huge thanks for all the reviews! @TossSky, the edge-case and policy testing was seriously impressive thank you. I've noted the two missing tests and will pick them up in a follow-up PR. Happy to see this one land!

[Aryam-21]

Branch updated with main. All review feedback has been addressed, and this already has an approving review + a PASS from @TossSky's testing. Just needs the pending workflow run approved to get CI green — could a maintainer approve it when you get a chance? 🙏