feat(mcp): add transactional move_file and delete_file edits

[tiendungdev]

Summary

• add move_file and delete_file variants to the atomic batch_edit transaction
• resolve and lock every source/destination path, refuse destination collisions, lifecycle overlap, symlink sources, non-regular files, and SHA-256 drift
• extend durable v1 receipts with explicit before/after absence state so interrupted moves and deletes can be classified and rolled back after restart
• refresh graph state for deleted sources and created destinations, with focused success, rollback, recovery, boundary, idempotency, and schema tests

Scope

This is a foundational slice toward #544, not a complete close of that issue.

Follow-up work intentionally left for focused PRs:

• Git classification (tracked, untracked, ignored, absent) and explicit ignored-path opt-in
• native atomic rename/cross-filesystem policy instead of the current durable write/remove transaction
• broader Windows/POSIX case and separator coverage plus user-facing documentation

Verification

• go test -race ./internal/mcp -run 'AtomicBatchLifecycle|AtomicBatchDurableRecovery|BatchEditItemKind|BatchEditItemsSchemaOneOf|AtomicBatchSameFile|AtomicBatchCommitFailure' -count=1
• go vet ./internal/mcp
• go build ./cmd/gortex
• git diff --cached --check

go test ./internal/mcp -count=1 still has unrelated Windows/baseline failures (URI/path expectations, symlink privilege, and existing graph fixture tests). Representative failures TestDiagnosticsBroadcaster_PayloadShape, TestMatchFidelityGlob, and TestNotesManager_SaveQueryDelete were reproduced unchanged on clean upstream/main at 858c80b5.

Part of #544

[zzet]

@tiendungdev thank you for the feature implementation. I found minor issues / nits worth fixing before the merge:

• Latent empty-file trap in the back-compat shim. In prepareBatchJournal, the !buffer.existenceSet fallback infers existence from buffer.original != nil / content != nil. For an empty file, append([]byte(nil), content...) with zero bytes yields nil, so that heuristic would misclassify an existing-but-empty file as absent. Production never hits it (readBatchBuffers always sets existenceSet=true), so it's only a trap for future direct-construction callers/tests. Consider dropping the heuristic and requiring existenceSet, or note it.
• mode vs fileMode redundancy on batchFileBuffer — mode is just fileMode.Perm(). The split (perm for writing, full mode for type checks) is legitimate but two overlapping fields invite drift; a one-line comment on why both exist would help.
• The move implements copy-then-remove (full content buffered in memory), not a native rename — the PR body already flags cross-filesystem/atomic-rename as deferred follow-up. Fine for this slice, but large-file moves cost 2× memory vs a rename.

Test coverage — strong, with gaps worth closing

Success, digest mismatch, destination-exists, rollback (fault-injected remove), symlink source, idempotent retry, durable writer, recovery, outside-root, invalid digest, and overlap are all covered. To fully lock the security story I'd add:

• a move_file whose destination is a symlink or sits under a symlinked parent directory (confirms destination-side guardSymlinkWithinRepo, currently only source-side and absolute-outside are tested);
• a ..-traversal destination refused;
• a delete_file on a directory refused (non-regular path).

These pass by construction given resolveFilePath, but tests would prevent a future refactor from silently regressing the destination guard.

[tiendungdev]

Addressed the review notes in 4ed7451: prepareBatchJournal now requires explicit existence state instead of inferring it from nil byte slices, mode/fileMode ownership is documented, and security coverage now includes destination symlink, symlinked destination parent, dot-dot traversal destination, and directory deletion refusal. Focused lifecycle tests and go vet ./internal/mcp pass locally; symlink cases follow the existing POSIX-only test posture.

[zzet]

@tiendungdev can you please resolve the merge conflicts?

[tiendungdev]

Resolved the merge conflicts in f34dddf by merging current upstream/main without force-pushing.

The conflict with 381f321 needed a semantic merge rather than marker removal: the new batch discriminator validation now accepts all four operations, keeps move_file/delete_file explicitly discriminated, rejects mixed lifecycle payloads before disk writes, and reports all four accepted shapes. The durability recovery fixtures from main now also set the explicit existence state required by the previously approved lifecycle invariant.

Verification:

• go test ./internal/mcp -run Test(ParseBatchEdits|BatchEdit|AtomicBatch) -count=1 -timeout=10m
• go vet ./internal/mcp
• git diff --cached --check

The focused lifecycle/transaction suite and vet pass. The full internal/mcp package still has unrelated Windows-only path, symlink-privilege, and index-fixture failures.

[tiendungdev]

Fixed the macOS CI failure in 8e8bf2c.

The destination guard now rejects a symlink at any component below the tracked repository root, both during snapshot planning and immediately before creating the destination. This closes the symlinked-parent case and the commit-time TOCTOU window while preserving repositories checked out through a symlinked parent prefix.

Verification:

• focused lifecycle destination/security tests
• broader ParseBatchEdits, BatchEdit, and AtomicBatch test set
• go vet ./internal/mcp
• git diff --check

[zzet]

The conflicts are resolved and the branch is mechanically ready — MERGEABLE/CLEAN, all checks green. I also merged current main into the head locally (the branch is 10 commits behind): no conflicts, builds clean, internal/mcp passes, and -race on the batch-transaction suite is clean. The four asks from my earlier review are addressed.

One regression to fix before this lands, plus two things worth folding in while you're in here.

Fix before merging

internal/mcp/batch_transaction.go:447 — the os.Stat → os.Lstat swap widens permissions on edit_file.

readBatchBuffers needed Lstat for the new fileMode, but buffer.mode was re-pointed at it too. edit_file has no symlink guard (the ModeSymlink check at :592 covers only move/delete), and resolveFilePath deliberately permits in-repo symlinks — so editing through a symlink now chmods the replacement to the link's bits rather than the target's.

Same test, both trees, one edit_file on link.txt -> target.txt where target is 0600:

merge-base d4801638:  AFTER link.txt  perm=0600   (target's bits preserved)
PR head    8e8bf2cd:  AFTER link.txt  perm=0755   (link's bits)

That's macOS. On Linux a symlink's st_mode is 0777, so this republishes a 0600 source file world-writable and world-executable. AtomicWriteFile does an explicit f.Chmod, so umask does not clamp it, and the bad mode is journaled at batch_transaction_journal.go:168 and re-applied by rollbackBatchReceipt.

It blocks where the rest don't because it is a regression of an op this PR never set out to change, it is silent, it has no coverage, and it contradicts the comment my earlier ask (b) requested — :41 says "permission bits preserved when writing replacement files", which for a symlink preserves the wrong file's bits.

Fix is one line: keep Lstat for fileMode, but take the perm bits from a followed stat.

if st, sErr := os.Stat(path); sErr == nil {
    buffer.mode = st.Mode().Perm()
}

Please add the A/B as a test — nothing currently pins it.

Worth folding in

:728 — commit creations before removals. orderedPaths is sort.Strings-ordered, so move aaa.txt -> zzz.txt runs remove(aaa) then write(zzz). Two consequences:

• A crash in that window leaves the file at neither path. That's a new failure class — before this PR a torn commit always left every file present with either the old or the new bytes.
• An out-of-process racer creating the destination between prepare and commit makes validateBatchCreateTarget fire after the source is already unlinked. rollbackBatchReceipt then classifies the destination as unknown and returns recovery_conflict at batch_transaction_journal.go:252 before restoring anything, so the source is gone from the working tree.

Two passes over orderedPaths — all existsAfter writes first, then all removals — closes both windows; with that order the racer aborts cleanly with the source untouched. Worth noting the guard has zero coverage today: deleting the whole if !buffer.existsBefore { validateBatchCreateTarget … } branch leaves go test ./internal/mcp fully green.

internal/mcp/tools_enhancements.go:298 — the tool contract still advertises two ops. The served description reads "Each edit is one of two operations selected by op" and bullets only edit_symbol and edit_file; the edits array description at :300 repeats it. grep move_file over docs/ returns nothing. The 4-branch oneOf schema carries the truth so the ops do work when called — but a slice whose point is to stop agents reaching for bash git mv shouldn't advertise that it can't. Two description literals plus a docs/mcp.md line.

[tiendungdev]

Addressed the latest review in db8ad1b.

• readBatchBuffers now keeps Lstat-derived fileMode for lifecycle type checks while taking replacement permissions from followed os.Stat, with a POSIX regression test for edit_file through a 0600 symlink target.
• The commit phase now writes every existsAfter image before removing any before-image. Focused tests pin create-before-remove ordering and the late destination collision guard.
• The served batch_edit description and edits field now advertise all four operations, and docs/mcp.md documents edit_symbol, edit_file, move_file, and delete_file.

Verification:

• go test ./internal/mcp -run Test(AtomicBatchEditFileThroughSymlinkPreservesTargetPermissions|AtomicBatchLifecycleCommitsCreationsBeforeRemovals|ValidateBatchCreateTargetRejectsLateDestination|BatchEditSchemaAdvertisesAtomicStatusProtocol)$ -count=1
• go test ./internal/mcp -run Test(ParseBatchEdits|BatchEdit|AtomicBatch) -count=1 -timeout=10m
• go test -race ./internal/mcp -run Test(ParseBatchEdits|BatchEdit|AtomicBatch) -count=1 -timeout=15m
• go vet ./internal/mcp
• go build ./cmd/gortex
• git diff --check

All checks pass locally. The symlink permission test follows the existing POSIX-only posture and is skipped on Windows.