Fix VFS removal diagnostics and Windows test portability

- Report Unsupported for unresolved network-mount symlink parents.
  - Clarify stable-tree requirements for Windows/SFTP recursive removal.
  - Derive delivery size budgets from platform type sizes.
  - Probe native long-path support before testing VFS publication.
  - Add ProxyCommand and asynchronous process teardown regressions.

docs/articles/vfs.md

@@ -156,13 +156,19 @@ symlinked destination is written through to its target. Providers report what th
 POSIX local recursive removal uses opened directory descriptors and never follows a replaced
 child symlink. It can partially remove a tree before reporting cancellation, an I/O error, or
 `ChangedExternally` when a directory replacement is detected. The supplied root's parent path
-is resolved when traversal starts; this is not a sandbox for untrusted root paths. Nested network
-mounts must be removed separately. Providers advertising `RecursiveRemove` provide their own
-directory-bound traversal. Other providers, including SFTP and the current Windows backend, use
-the generic pathname walker: symlinks present when inspected are removed themselves, but a
-concurrent directory/link replacement can redirect later child operations. Coordinate writers
-before recursive removal on those providers; do not use that fallback to delete an untrusted,
-concurrently modified tree.
+is resolved when traversal starts; this is not a sandbox for untrusted root paths. An unresolved
+symlink in the immediate parent inside a network mount returns `Unsupported`; use the real
+directory path. Nested network mounts must be removed separately. Providers advertising
+`RecursiveRemove` provide their own directory-bound traversal. Other providers, including SFTP
+and the current Windows backend, use the generic pathname walker. Inspected symlinks are removed
+themselves, but concurrent directory/link replacement can redirect later child operations.
+Coordinate writers before recursive removal on those providers; do not use that fallback to
+delete an untrusted, concurrently modified tree.
+
+Applications requiring directory-bound recursive removal should check
+`vfs.capabilities( uri ).has( VFSCapability::RecursiveRemove )` before calling `remove()`.
+The generic Windows/SFTP fallback remains available for stable trees; it does not promise
+protection against concurrent directory or link replacement.
 
 ### Copy and move
 

src/modules/vfs/include/eepp/vfs/options.hpp

@@ -68,7 +68,8 @@ struct EE_VFS_API MkDirOptions {
 
 struct EE_VFS_API RemoveOptions {
 	/** Remove directory contents recursively. Passed to providers with RecursiveRemove;
-	 * otherwise VirtualFileSystem walks using pathname operations (not race resistant). */
+	 * otherwise VirtualFileSystem walks using pathname operations. That fallback requires a
+	 * stable tree: coordinate other writers; it is not safe against concurrent link replacement. */
 	bool recursive{ false };
 };
 

src/modules/vfs/include/eepp/vfs/virtualfilesystem.hpp

@@ -141,8 +141,10 @@ class EE_VFS_API VirtualFileSystem {
 	VFSTask<void> makeDirs( const URI& uri, const MkDirOptions& options = {},
 							CancellationToken token = {} );
 
-	/** Removes an entry. Directories require options.recursive unless empty. Symlinks are
-	 * removed, never followed. */
+	/** Removes an entry. Directories require options.recursive unless empty. Inspected symlinks
+	 * are removed themselves. Recursive removal without RecursiveRemove uses pathname traversal:
+	 * callers must prevent concurrent directory/link replacement (Windows local and SFTP included).
+	 * Check capabilities(uri) for RecursiveRemove when directory-bound traversal is required. */
 	VFSTask<void> remove( const URI& uri, const RemoveOptions& options = {},
 						  CancellationToken token = {} );
 

src/modules/vfs/src/eepp/vfs/providers/localproviderposix.cpp

@@ -686,7 +686,8 @@ namespace {
  * redirect subsequent child operations. Diagnostic paths are never used for child syscalls. */
 class LocalTreeRemover {
   public:
-	static VFSResult<void> remove( std::string path, const CancellationToken& token ) {
+	static VFSResult<void> remove( std::string path, const CancellationToken& token,
+								   bool networkMount ) {
 		while ( path.size() > 1 && path.back() == '/' )
 			path.pop_back();
 		const size_t slash = path.find_last_of( '/' );
@@ -697,7 +698,8 @@ class LocalTreeRemover {
 		}
 		// Resolve only the supplied parent for mount routing/diagnostics. Descendant operations
 		// below are descriptor-relative and never resolve their mutable pathname again.
-		path = LocalIOScheduler::resolvePath( path, false );
+		if ( !networkMount )
+			path = LocalIOScheduler::resolvePath( path, false );
 		const size_t parentSlash = path.find_last_of( '/' );
 		const std::string parent = parentSlash == std::string::npos
 									   ? "."
@@ -715,8 +717,18 @@ class LocalTreeRemover {
 		do {
 			fd = ::open( parent.c_str(), flags );
 		} while ( fd < 0 && errno == EINTR );
-		if ( fd < 0 )
-			return Err( errnoError( path, "Cannot open removal parent" ) );
+		if ( fd < 0 ) {
+			const int error = errno;
+			struct stat metadata;
+			if ( ( error == ELOOP || error == ENOTDIR ) &&
+				 ::lstat( parent.c_str(), &metadata ) == 0 && S_ISLNK( metadata.st_mode ) ) {
+				return Err( VFSError( VFSErrorCode::Unsupported, URI::fromFSPath( path ),
+									  "Recursive removal requires a resolved parent; use the real "
+									  "directory path instead of a network-mount symlink",
+									  error ) );
+			}
+			return Err( errnoError( path, "Cannot open removal parent", error ) );
+		}
 		struct Parent {
 			int fd;
 
@@ -824,9 +836,11 @@ class LocalTreeRemover {
 VFSTask<void> LocalProvider::remove( const URI& uri, const RemoveOptions& options,
 									 CancellationToken token ) {
 	if ( options.recursive ) {
-		return run<void>( uri, LastLink::Keep, token, [token]( const std::string& path ) {
-			return LocalTreeRemover::remove( path, token );
-		} );
+		return run<void>( uri, LastLink::Keep, token,
+						  [scheduler = mScheduler, token]( const std::string& path ) {
+							  return LocalTreeRemover::remove(
+								  path, token, scheduler->routeFor( path ).isNetworkMount() );
+						  } );
 	}
 	return run<void>(
 		uri, LastLink::Keep, std::move( token ), []( const std::string& path ) -> VFSResult<void> {

src/modules/vfs/src/eepp/vfs/virtualfilesystem.cpp

@@ -460,7 +460,9 @@ VFSResult<void> removeTree( VFSProvider& provider, const URI& uri, const Cancell
 					return Err( VFSError( VFSErrorCode::ProtocolError, uri,
 										  "Invalid directory entry name" ) );
 				}
-				// Symlinks are removed, never followed.
+				// Remove inspected links themselves. This pathname fallback requires a stable tree:
+				// a later directory/link replacement can redirect openDirectory or child
+				// operations.
 				VFSResult<void> removed =
 					entry.type == FileType::Directory
 						? removeTree( provider, uri.withAppendedSegment( entry.name ), token,

src/tests/unit_tests/coalesceddelivery_tests.cpp

@@ -377,5 +377,10 @@ UTEST( CoalescedDelivery, concurrentPublicationAndCloseStress ) {
 		EXPECT_EQ( 0, afterClose.load() );
 	}
 	printf( "  CoalescedDelivery<int>: %zu bytes\n", sizeof( Delivery ) );
-	EXPECT_LE( sizeof( Delivery ), 256u );
+	// Standard-library synchronization types have different sizes across ABIs. Budget those
+	// separately, leaving 64 bytes for the payload, pointers, counters, flags and padding.
+	constexpr size_t budget = sizeof( std::mutex ) + sizeof( std::condition_variable ) +
+							  sizeof( Delivery::Callback ) +
+							  sizeof( std::enable_shared_from_this<Delivery> ) + 64;
+	EXPECT_LE( sizeof( Delivery ), budget );
 }

src/tests/unit_tests/process_tests.cpp

@@ -1,7 +1,7 @@
 #include <eepp/config.hpp>
 
-#if EE_PLATFORM == EE_PLATFORM_LINUX || EE_PLATFORM == EE_PLATFORM_MACOS || \
-	EE_PLATFORM == EE_PLATFORM_BSD
+#if EE_PLATFORM == EE_PLATFORM_WIN || EE_PLATFORM == EE_PLATFORM_LINUX || \
+	EE_PLATFORM == EE_PLATFORM_MACOS || EE_PLATFORM == EE_PLATFORM_BSD
 
 #include "utest.hpp"
 #include "vfs_test_utils.hpp"
@@ -9,15 +9,54 @@
 #include <condition_variable>
 #include <cstdlib>
 #include <eepp/system/process.hpp>
+#include <optional>
+#if EE_PLATFORM != EE_PLATFORM_WIN
 #include <fcntl.h>
 #include <pthread.h>
 #include <signal.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#endif
 
 using namespace vfstest;
 using EE::System::Process;
 
+UTEST( Process, asyncReaderTeardownDoesNotWaitForChildInput ) {
+	std::mutex mutex;
+	std::condition_variable changed;
+	std::string output;
+	std::optional<Process> process( std::in_place );
+#if EE_PLATFORM == EE_PLATFORM_WIN
+	const std::string shell = "cmd.exe";
+	const std::vector<std::string> arguments{ "/D", "/Q", "/C", "echo ready&set /p token=" };
+#else
+	const std::string shell = "/bin/sh";
+	const std::vector<std::string> arguments{ "-c", "trap '' TERM; printf ready; read token" };
+#endif
+	ASSERT_TRUE( process->create( shell, arguments,
+								  Process::getDefaultOptions() | Process::EnableAsync |
+									  Process::CombinedStdoutStderr ) );
+	process->startAsyncRead( [&]( const char* bytes, size_t count ) {
+		std::lock_guard<std::mutex> lock( mutex );
+		output.append( bytes, count );
+		changed.notify_all();
+	} );
+	{
+		std::unique_lock<std::mutex> lock( mutex );
+		ASSERT_TRUE( changed.wait_for( lock, std::chrono::seconds( 2 ), [&] {
+			return output.find( "ready" ) != std::string::npos;
+		} ) );
+	}
+	ASSERT_TRUE( process->isAlive() );
+	const auto started = std::chrono::steady_clock::now();
+	process.reset();
+	const auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
+		std::chrono::steady_clock::now() - started );
+	printf( "  Async process teardown: %lld ms\n", static_cast<long long>( elapsed.count() ) );
+	EXPECT_LT( elapsed.count(), 1000 );
+}
+
+#if EE_PLATFORM != EE_PLATFORM_WIN
 UTEST( Process, joinRetriesInterruptedWaits ) {
 	TempDir temp;
 	const std::string fifo = temp.path + "release";
@@ -83,3 +122,5 @@ UTEST( Process, joinRetriesInterruptedWaits ) {
 }
 
 #endif
+
+#endif

src/tests/unit_tests/sftp_tests.cpp

@@ -1105,6 +1105,75 @@ UTEST( SFTP, proxyJumpAndProxyCommand ) {
 					 readString( fx.vfs, server.uri( "proxied", name + "/x.txt" ) ) );
 }
 
+UTEST( SFTP, blackholedProxyCommandTeardownKeepsOtherSessionsResponsive ) {
+	REQUIRE_SSHD();
+	if ( Sys::which( "nc" ).empty() )
+		UTEST_SKIP( "netcat is required for the ProxyCommand teardown test" );
+	BlackholeProxy proxy( server.port() );
+	ASSERT_NE( 0, proxy.port() );
+	TempDir temp;
+	std::string original;
+	ASSERT_TRUE( FileSystem::fileGet( server.dir() + "ssh_config", original ) );
+	const std::string configuration =
+		"Host shutdown-proxy\n    ProxyCommand trap '' TERM; exec nc 127.0.0.1 " +
+		std::to_string( proxy.port() ) + "\n" + original;
+	ASSERT_TRUE( FileSystem::fileWrite( temp.path + "config", std::string_view( configuration ) ) );
+	SFTPProviderConfig config;
+	config.watchAgentCommand.clear();
+	config.configContextFactory = [path = temp.path]( const std::string& host ) {
+		auto context = SSHConfigContext::system( host );
+		context.sources = { { path + "config", path } };
+		return context;
+	};
+	auto impl = std::make_shared<sftp::SFTPProviderImpl>( std::move( config ) );
+	const URI proxied = server.uri( "shutdown-proxy" );
+	const URI direct = server.uri( "testhost" );
+	auto tunnel = waitFor( impl->connection( proxied ) );
+	REQUIRE_OK( tunnel );
+	auto live = waitFor( impl->connection( direct ) );
+	REQUIRE_OK( live );
+	ASSERT_TRUE( tunnel.value() != live.value() );
+	proxy.drop( true );
+	const auto started = std::chrono::steady_clock::now();
+	tunnel.value()->close();
+	// The second context turn runs after the first session's transport has been destroyed.
+	TaskResolver<VFSResult<bool>> resolve;
+	auto retired = VFSTask<bool>::create( resolve );
+	auto context = impl->ioContext();
+	context->post( [context, connection = tunnel.value(),
+					resolve = std::move( resolve )]() mutable {
+		context->post( [connection = std::move( connection ),
+						resolve = std::move( resolve )]() mutable {
+			resolve( Ok( connection->session() == nullptr && connection->transport() == nullptr ) );
+		} );
+	} );
+	ASSERT_EQ( TaskState::Ready, retired.waitFor( std::chrono::seconds( 2 ) ) );
+	auto done = waitFor( std::move( retired ) );
+	REQUIRE_OK( done );
+	EXPECT_TRUE( done.value() );
+	const std::string path = server.remoteRoot();
+	auto stat = live.value()->submit<void>(
+		{}, direct, [path]( sftp::SSHConnection& connection ) -> std::optional<VFSResult<void>> {
+			LIBSSH2_SFTP_ATTRIBUTES attributes{};
+			const int result = libssh2_sftp_stat_ex( connection.sftp(), path.c_str(),
+													 static_cast<unsigned>( path.size() ),
+													 LIBSSH2_SFTP_STAT, &attributes );
+			if ( result == LIBSSH2_ERROR_EAGAIN )
+				return std::nullopt;
+			if ( result != 0 )
+				return Err( connection.lastError( URI(), "Independent session stalled" ) );
+			return Ok();
+		} );
+	ASSERT_EQ( TaskState::Ready, stat.waitFor( std::chrono::seconds( 2 ) ) );
+	auto response = waitFor( std::move( stat ) );
+	REQUIRE_OK( response );
+	const auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
+		std::chrono::steady_clock::now() - started );
+	printf( "  ProxyCommand retirement and independent stat: %lld ms\n",
+			static_cast<long long>( elapsed.count() ) );
+	EXPECT_LT( elapsed.count(), 1000 );
+}
+
 UTEST( SFTP, connectionReuseReconnectAndErrors ) {
 	REQUIRE_SSHD();
 	SFTPFixture fx;

src/tests/unit_tests/vfs_tests.cpp

@@ -1,3 +1,15 @@
+#include <eepp/config.hpp>
+
+#if EE_PLATFORM == EE_PLATFORM_WIN
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN
+#endif
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+#include <windows.h>
+#endif
+
 #include "utest.hpp"
 #include "vfs_test_utils.hpp"
 #include <atomic>
@@ -150,6 +162,44 @@ UTEST( VFSProvider, localMetadataAndRevisions ) {
 }
 
 #if EE_PLATFORM != EE_PLATFORM_WIN
+UTEST( VFSLocal, recursiveRemovalThroughParentSymlinksHasAnExplicitMountPolicy ) {
+	TempDir tmp;
+	VirtualFileSystem local;
+	auto canonical = waitFor( local.canonicalize( tmp.uri() ) );
+	ASSERT_TRUE( canonical );
+	const URI root = std::move( canonical ).value();
+	const URI real = root.withAppendedSegment( "real" );
+	const URI alias = root.withAppendedSegment( "alias" );
+	ASSERT_TRUE( waitFor( local.makeDir( real ) ) );
+	ASSERT_EQ( 0, ::symlink( real.getFSPath().c_str(), alias.getFSPath().c_str() ) );
+	const URI tree = real.withAppendedSegment( "tree" );
+	const URI viaLink = alias.withAppendedSegment( "tree" );
+	ASSERT_TRUE( waitFor( local.makeDir( tree ) ) );
+	ASSERT_TRUE( writeString( local, tree.withAppendedSegment( "file" ), "keep" ) );
+	ASSERT_TRUE( waitFor( local.remove( viaLink, RemoveOptions{ true } ) ) );
+	EXPECT_EQ( VFSErrorCode::NotFound, codeOf( waitFor( local.stat( tree ) ) ) );
+	ASSERT_TRUE( waitFor( local.makeDir( tree ) ) );
+	ASSERT_TRUE( writeString( local, tree.withAppendedSegment( "file" ), "keep" ) );
+	const std::string mount = root.getFSPath();
+	auto pool = ThreadPool::createShared( 1 );
+	auto scheduler = std::make_shared<LocalIOScheduler>(
+		pool, Seconds( 2 ),
+		[mount]( const std::string& path ) -> std::optional<LocalIOScheduler::NetworkMount> {
+			if ( path.starts_with( mount ) )
+				return LocalIOScheduler::NetworkMount{ mount, "testserver:/share" };
+			return std::nullopt;
+		} );
+	VirtualFileSystem::Config config;
+	config.registerDefaultProviders = false;
+	VirtualFileSystem network( config );
+	network.registerProvider( std::make_shared<LocalProvider>( scheduler ) );
+	auto removed = waitFor( network.remove( viaLink, RemoveOptions{ true } ) );
+	EXPECT_EQ( VFSErrorCode::Unsupported, codeOf( removed ) );
+	EXPECT_STDSTREQ( "keep", readString( local, tree.withAppendedSegment( "file" ) ) );
+	// The real directory remains removable on the same network lane.
+	ASSERT_TRUE( waitFor( network.remove( tree, RemoveOptions{ true } ) ) );
+}
+
 UTEST( VFSLocal, recursiveRemovalDoesNotUsePathnameTraversal ) {
 	TempDir tmp;
 	VirtualFileSystem vfs;
@@ -1675,19 +1725,38 @@ UTEST( VFSLocal, atomicWritesAndCopiesSupportLongBasenames ) {
 	TempDir tmp;
 	VirtualFileSystem vfs;
 	const URI source = tmp.uri().withAppendedSegment( "source" );
+	bool tested = false;
 	for ( size_t length : { 240u, 250u } ) {
 		ASSERT_TRUE( writeString( vfs, source, "copied" ) );
 		const URI file = tmp.uri().withAppendedSegment( std::string( length, 'a' ) );
-		// Establish that this filesystem/configuration supports the destination itself.
+#if EE_PLATFORM == EE_PLATFORM_WIN
+		// A native open is the prerequisite: legacy MAX_PATH failures may be reported as
+		// ERROR_PATH_NOT_FOUND instead of ERROR_FILENAME_EXCED_RANGE on Windows runners.
+		const auto native = String::fromUtf8( file.getFSPath() ).toWideString();
+		HANDLE handle = CreateFileW( native.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_NEW,
+									 FILE_ATTRIBUTE_NORMAL, nullptr );
+		if ( handle == INVALID_HANDLE_VALUE ) {
+			const DWORD error = GetLastError();
+			printf( "  Native long-path probe: error=%lu, path length=%zu\n",
+					static_cast<unsigned long>( error ), native.size() );
+			if ( native.size() >= MAX_PATH &&
+				 ( error == ERROR_FILENAME_EXCED_RANGE || error == ERROR_PATH_NOT_FOUND ||
+				   error == ERROR_INVALID_NAME ) )
+				continue;
+			ASSERT_TRUE( handle != INVALID_HANDLE_VALUE );
+		}
+		ASSERT_TRUE( CloseHandle( handle ) );
+		ASSERT_TRUE( DeleteFileW( native.c_str() ) );
+#endif
+		// Once the platform can create the destination, every VFS failure is a regression.
 		auto probe = waitFor(
 			vfs.open( file, OpenFlags::Write | OpenFlags::Create | OpenFlags::Exclusive ) );
-#if EE_PLATFORM == EE_PLATFORM_WIN
-		if ( !probe && probe.error().is( VFSErrorCode::InvalidURI ) ) {
-			printf( "  Windows long paths unavailable: %s\n", probe.error().toString().c_str() );
-			continue;
+		if ( !probe ) {
+			printf( "  VFS long-path probe: %s (native=%lld)\n", probe.error().toString().c_str(),
+					static_cast<long long>( probe.error().nativeCode ) );
 		}
-#endif
 		ASSERT_TRUE( probe );
+		tested = true;
 		ASSERT_TRUE( waitFor( probe.value().close() ) );
 		ASSERT_TRUE( writeString( vfs, file, "atomic save" ) );
 		EXPECT_STDSTREQ( "atomic save", readString( vfs, file ) );
@@ -1711,6 +1780,8 @@ UTEST( VFSLocal, atomicWritesAndCopiesSupportLongBasenames ) {
 		ASSERT_TRUE( listed );
 		EXPECT_TRUE( sortedNames( listed.value() ) == std::vector<std::string>( { "source" } ) );
 	}
+	if ( !tested )
+		UTEST_SKIP( "Native filesystem/API configuration cannot create long destination paths" );
 }
 
 #if EE_PLATFORM != EE_PLATFORM_WIN