fix(remotefs): inspect the stat error in WinFS Remove and RemoveAll

[kke]

Both decided whether a path is a directory with err == nil on a Stat call and never looked at the error, so any stat failure read as "not a directory". For RemoveAll on a populated directory that skipped the recursive delete, fell through to Remove, and ran the non-recursive rmdir instead: the tree survived and the caller was told "the directory is not empty" by the one function whose purpose is deleting non-empty directories, with nothing in the error mentioning the connection.

Both now branch on what the error actually says, following the os package:

• path is a file -> del
• path is a directory -> rmdir, recursive for RemoveAll
• path does not exist -> error for Remove, nil for RemoveAll
• stat failed at transport -> propagate, never fall through to del

The del is factored into removeFile so RemoveAll no longer reaches it via Remove, which re-stat'ed the same path.

Behaviour change: Remove on a missing path is now an error, matching os.Remove, where del previously exited zero and the call returned nil. Every in-tree caller either discards that error or removes a path it just created.

Prerequisite carried along: this also contains the one-line WinFS.Stat fix from #427, which stops an execution failure being reported as fs.ErrNotExist. It is required here rather than optional -- RemoveAll now treats ErrNotExist as "nothing to remove", so without it a dropped connection would return nil and turn today's loud failure into a silent one. Drop this hunk if #427 has already landed.

Fixes #428

[Copilot]

Pull request overview

This PR fixes incorrect directory/file handling in the Windows remote filesystem implementation by ensuring WinFS.Remove and WinFS.RemoveAll properly inspect Stat errors (instead of treating any stat failure as “not a directory”), and by correcting WinFS.Stat so transport/exec failures are not misclassified as fs.ErrNotExist.

Changes:

• Update WinFS.Stat to propagate command-execution/transport errors without wrapping them as fs.ErrNotExist.
• Rework WinFS.Remove and WinFS.RemoveAll to branch on Stat outcomes (dir/file/missing/transport failure) and avoid incorrect fallthrough behavior.
• Add focused unit tests covering missing-path semantics, directory recursion behavior, and transport failures for Remove/RemoveAll.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
remotefs/winfs.go	Fixes Stat error classification and corrects Remove/RemoveAll branching to match os.Remove/os.RemoveAll semantics on Windows.
remotefs/winfs_test.go	Adds regression tests for the corrected Stat/Remove/RemoveAll behavior, including transport-failure cases.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

remotefs/writefileatomic_test.go:90

• Test comment is now inaccurate: the cleanup defer calls RemoveAll (not Remove), so the explanatory comment should be updated to match the new behavior.

		// After a successful rename the temp path no longer exists; Remove is
		// still called by defer but the error is ignored.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

remotefs/winfs.go:120

• WinFS.Remove currently wraps Stat errors with fmt.Errorf("remove %s: %w", ...). When the path is missing (or Stat fails), callers doing errors.As(err, *fs.PathError) will see Op="stat" (from Stat) rather than Op="remove", which diverges from os.Remove semantics and from PosixFS.Remove (which returns a PathError with Op="remove" for ErrNotExist). Consider returning a PathError from Remove so the top-level operation matches the API contract while still preserving the underlying cause in the error chain.

	if err != nil {
		// Covers both a path that is genuinely absent -- an error for os.Remove --
		// and a stat that could not be performed. Neither may fall through to del:
		// a transport failure is not evidence of anything about the path.
		return fmt.Errorf("remove %s: %w", name, err)

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.