Fix bulk upload progress not reaching 100%

.github/ISSUE_TEMPLATE/bug_report.md

@@ -6,46 +6,37 @@ labels: bug
 assignees: ''
 ---
 
-**Please do not report security vulnerabilities here.** See [SECURITY.md](../../SECURITY.md) for the private reporting process.
+> [!WARNING]
+> **Please do not report security vulnerabilities here.** See [SECURITY.md](../../SECURITY.md) for the private reporting process.
+
+> [!INFO]
+> Don't worry about filling out every field, anything you can provide helps. Only the description really matters; the rest is optional. I care to know about issues, even if you don't write an essay about the problem.
 
 ## Describe the bug
 A clear, concise description of what's going wrong.
 
-## Steps to reproduce
+## Steps to reproduce (optional)
 1.
 2.
 3.
 
-## Expected behavior
+## Expected behavior (optional)
 What you expected to happen instead.
 
-## Actual behavior
-What actually happened. Include exact error messages if any.
-
-## Screenshots or recordings
+## Screenshots or recordings (optional)
 If applicable, attach screenshots or a short screen recording. Drag and drop works.
 
-## Environment
+## Environment (optional)
 - **nui-sftp version:** (e.g. 0.4.2 — see `nui-sftp --version` or About dialog)
 - **Install source:** (e.g. AUR, built from source, AppImage, release binary)
 - **OS and version:** (e.g. Arch Linux, Ubuntu 24.04, Windows 11)
-- **Desktop environment / WM:** (e.g. KDE Plasma 6.1 on Wayland, GNOME 46 on X11) — Linux only
-- **Architecture:** (e.g. x86_64, aarch64)
 
-## Remote server (if relevant)
-- **SSH server:** (e.g. OpenSSH 9.6, Dropbear, proprietary appliance)
-- **Authentication method:** (password, public key, agent, certificate)
-- **Connection type:** (direct, via ProxyJump, through a tunnel)
-
-## Logs
+## Logs (optional)
 Logs can be found at:
 - **Linux:** `~/.local/state/nui-sftp/logs/`
 - **Windows:** `C:\Users\<your-username>\Documents\nui-sftp\logs\`
 
-Please paste the relevant log output below. **Redact hostnames, usernames, IPs, key fingerprints, and anything else sensitive** before sharing. 
+Please paste the relevant log output below. **Redact hostnames, usernames, IPs, key fingerprints, and anything else sensitive** before sharing.
 
 <details>
 </details>
-
-## Additional context
-Anything else worth knowing — frequency of the bug, recent changes to your setup, related issues, workarounds you've tried.

backend/source/backend/session.cpp

@@ -9,6 +9,10 @@
 
 #include <nui/utility/scope_exit.hpp>
 
+#include <optional>
+#include <unordered_map>
+#include <unordered_set>
+
 using namespace std::chrono_literals;
 
 Session::Session(
@@ -189,8 +193,7 @@ void Session::adoptBulkResumes(std::vector<Ids::OperationId> const& operationIds
             if (!sftp)
             {
                 Log::warn(
-                    "Session::adoptBulkResumes: no sftp channel open yet — deferring {} resume(s)",
-                    operationIds.size()
+                    "Session::adoptBulkResumes: no sftp channel open yet — deferring {} resume(s)", operationIds.size()
                 );
                 return;
             }
@@ -201,8 +204,7 @@ void Session::adoptBulkResumes(std::vector<Ids::OperationId> const& operationIds
                 if (!entry)
                 {
                     Log::info(
-                        "Session::adoptBulkResumes: no backup for operation '{}' (already evicted?)",
-                        opId.value()
+                        "Session::adoptBulkResumes: no backup for operation '{}' (already evicted?)", opId.value()
                     );
                     continue;
                 }
@@ -806,8 +808,7 @@ void Session::registerRpcSftpAddArchiveDownloadOperation()
                 std::vector<SharedData::DirectoryEntry> entries;
                 try
                 {
-                    entries = nlohmann::json::parse(entriesJson)
-                        .get<std::vector<SharedData::DirectoryEntry>>();
+                    entries = nlohmann::json::parse(entriesJson).get<std::vector<SharedData::DirectoryEntry>>();
                 }
                 catch (std::exception const& exc)
                 {
@@ -929,14 +930,10 @@ void Session::registerRpcSftpAddArchiveUploadOperation()
                                 remoteArchivePath,
                                 result.error().toString()
                             );
-                            return reply.error(
-                                "Failed to add archive-upload operation: " + result.error().toString()
-                            );
+                            return reply.error("Failed to add archive-upload operation: " + result.error().toString());
                         }
                         Log::info(
-                            "Added archive-upload operation '{}' → '{}'",
-                            newOperationIdString,
-                            remoteArchivePath
+                            "Added archive-upload operation '{}' → '{}'", newOperationIdString, remoteArchivePath
                         );
                         self->resetQueueThrottle();
                         reply({{"success", true}});
@@ -967,9 +964,7 @@ void Session::registerRpcSftpAddBulkDownloadOperation()
 
                 // Expect N per-entry ids + 1 dedicated aggregate-bulk-card id at the end.
                 if (operationIdStrings.size() != request.entries.size() + 1)
-                    return reply.error(
-                        "addBulkDownload: operationIds and entries length mismatch"
-                    );
+                    return reply.error("addBulkDownload: operationIds and entries length mismatch");
 
                 self->withSftpChannelDo(
                     Ids::makeChannelId(channelIdString),
@@ -985,17 +980,14 @@ void Session::registerRpcSftpAddBulkDownloadOperation()
                         const auto enqueued = self->operationQueue_->addBulkDownloadOperation(
                             *channel,
                             request,
-                            [&operationIdStrings](std::size_t idx) {
+                            [&operationIdStrings](std::size_t idx)
+                            {
                                 return Ids::makeOperationId(operationIdStrings[idx]);
                             },
                             bulkCardId
                         );
 
-                        Log::info(
-                            "addBulkDownload: queued {}/{} entries",
-                            enqueued,
-                            request.entries.size()
-                        );
+                        Log::info("addBulkDownload: queued {}/{} entries", enqueued, request.entries.size());
 
                         self->resetQueueThrottle();
                         reply({{"success", true}, {"enqueued", enqueued}});
@@ -1039,7 +1031,8 @@ void Session::registerRpcSftpAddBulkUploadOperation()
                         const auto enqueued = self->operationQueue_->addBulkUploadOperation(
                             *channel,
                             request,
-                            [&operationIdStrings](std::size_t idx) {
+                            [&operationIdStrings](std::size_t idx)
+                            {
                                 return Ids::makeOperationId(operationIdStrings[idx]);
                             },
                             bulkCardId
@@ -1315,10 +1308,7 @@ void Session::registerRpcSftpRecomputeSyncDiff()
                                         // Matches the OperationQueue::rpcName scheme
                                         // the frontend OperationQueue listens on.
                                         parent->hub_->callRemote(
-                                            fmt::format(
-                                                "OperationQueue::{}::onSyncDiffProgress",
-                                                parent->id_.value()
-                                            ),
+                                            fmt::format("OperationQueue::{}::onSyncDiffProgress", parent->id_.value()),
                                             syncSessionId,
                                             std::to_string(compared)
                                         );
@@ -1617,9 +1607,7 @@ void Session::registerRpcSftpAddBulkDeleteOperation()
                             return reply({{"error", "Session no longer exists"}});
 
                         const auto enqueued = self->operationQueue_->addBulkDeleteOperation(
-                            *channel,
-                            request,
-                            Ids::makeOperationId(bulkOperationIdString)
+                            *channel, request, Ids::makeOperationId(bulkOperationIdString)
                         );
 
                         Log::info("addBulkDelete: queued {} entries", enqueued);
@@ -1642,9 +1630,7 @@ void Session::registerRpcSftpExistsBatch()
     on(fmt::format("Session::{}::sftp::existsBatch", id_.value()))
         .perform(
             [weak = weak_from_this()](
-                RpcHelper::RpcOnce&& reply,
-                std::string const& channelIdString,
-                std::vector<std::string> const& paths
+                RpcHelper::RpcOnce&& reply, std::string const& channelIdString, std::vector<std::string> const& paths
             )
             {
                 auto self = weak.lock();
@@ -1655,23 +1641,86 @@ void Session::registerRpcSftpExistsBatch()
                     Ids::makeChannelId(channelIdString),
                     [paths](RpcHelper::RpcOnce&& reply, auto&& channel)
                     {
-                        std::vector<bool> results;
-                        results.reserve(paths.size());
-                        for (auto const& path : paths)
+                        // Small drops: a handful of stat round-trips is cheaper than reading
+                        // a potentially huge target directory in full. Above the threshold the
+                        // single readdir below wins (one round-trip instead of N).
+                        constexpr std::size_t listingThreshold = 10;
+                        if (paths.size() < listingThreshold)
+                        {
+                            std::vector<bool> results;
+                            results.reserve(paths.size());
+                            for (auto const& path : paths)
+                            {
+                                auto fut = channel->stat(Utility::pathFromUtf8(path));
+                                if (fut.wait_for(futureTimeout) != std::future_status::ready)
+                                {
+                                    Log::warn(
+                                        "sftp::existsBatch: stat timeout for '{}' (treating as not-exists)", path
+                                    );
+                                    results.push_back(false);
+                                    continue;
+                                }
+                                const auto result = fut.get();
+                                results.push_back(result.has_value());
+                            }
+                            Log::info("sftp::existsBatch: probed {} paths via stat", paths.size());
+                            reply({{"success", true}, {"exists", results}});
+                            return;
+                        }
+
+                        // Larger drops: group destinations by parent directory and list each
+                        // parent once (readdir) rather than issuing one stat round-trip per
+                        // path. A bulk drop targets a single directory, so this collapses N
+                        // sequential SFTP round-trips into a single listing — a shallow
+                        // existence diff, like sync does. A listing failure (e.g. the parent
+                        // doesn't exist yet) degrades to "nothing exists", matching the
+                        // per-stat fallback.
+                        std::unordered_map<std::string, std::optional<std::unordered_set<std::string>>> listingByParent;
+
+                        auto listParent = [&channel](
+                                              std::filesystem::path const& parent
+                                          ) -> std::optional<std::unordered_set<std::string>>
                         {
-                            auto fut = channel->stat(Utility::pathFromUtf8(path));
+                            auto fut = channel->listDirectory(parent);
                             if (fut.wait_for(futureTimeout) != std::future_status::ready)
                             {
                                 Log::warn(
-                                    "sftp::existsBatch: stat timeout for '{}' (treating as not-exists)", path
+                                    "sftp::existsBatch: listing timed out for '{}' (treating contents as not-exists)",
+                                    parent.generic_string()
                                 );
-                                results.push_back(false);
-                                continue;
+                                return std::nullopt;
                             }
                             const auto result = fut.get();
-                            results.push_back(result.has_value());
+                            if (!result.has_value())
+                                return std::nullopt;
+                            std::unordered_set<std::string> names;
+                            names.reserve(result->size());
+                            for (auto const& entry : *result)
+                                names.insert(entry.path.filename().generic_string());
+                            return names;
+                        };
+
+                        std::vector<bool> results;
+                        results.reserve(paths.size());
+                        for (auto const& path : paths)
+                        {
+                            const auto fsPath = Utility::pathFromUtf8(path);
+                            const auto parentKey = fsPath.parent_path().generic_string();
+                            auto it = listingByParent.find(parentKey);
+                            if (it == listingByParent.end())
+                                it = listingByParent.emplace(parentKey, listParent(fsPath.parent_path())).first;
+
+                            auto const& listing = it->second;
+                            results.push_back(
+                                listing.has_value() &&
+                                listing->find(fsPath.filename().generic_string()) != listing->end()
+                            );
                         }
-                        Log::info("sftp::existsBatch: probed {} paths", paths.size());
+                        Log::info(
+                            "sftp::existsBatch: probed {} paths across {} dir listing(s)",
+                            paths.size(),
+                            listingByParent.size()
+                        );
                         reply({{"success", true}, {"exists", results}});
                     },
                     std::move(reply)

backend/source/backend/sftp/bulk_download_operation.cpp

@@ -120,14 +120,7 @@ std::expected<BulkDownloadOperation::WorkStatus, BulkDownloadOperation::Error> B
                 if (!prescannedPathOverride_.empty())
                 {
                     options_.overallProgressCallback(
-                        options_.localPath,
-                        currentIndex_,
-                        entries_.size(),
-                        0,
-                        0,
-                        currentBytes_,
-                        totalBytes_,
-                        bulkBytesPerSecond_
+                        options_.localPath, currentIndex_, entries_.size(), 0, 0, currentBytes_, totalBytes_, 0
                     );
                 }
                 Log::info("BulkDownloadOperation: Bulk download completed.");
@@ -370,7 +363,7 @@ void BulkDownloadOperation::completeCurrentDownload()
     if (!prescannedPathOverride_.empty() && currentIndex_ == entries_.size())
     {
         options_.overallProgressCallback(
-            options_.localPath, currentIndex_, entries_.size(), 0, 0, currentBytes_, totalBytes_, bulkBytesPerSecond_
+            options_.localPath, currentIndex_, entries_.size(), 0, 0, currentBytes_, totalBytes_, 0
         );
     }
 }

backend/source/backend/sftp/bulk_upload_operation.cpp

@@ -113,14 +113,7 @@ std::expected<BulkUploadOperation::WorkStatus, BulkUploadOperation::Error> BulkU
                 if (!prescannedPathOverride_.empty())
                 {
                     options_.overallProgressCallback(
-                        options_.remotePath,
-                        currentIndex_,
-                        entries_.size(),
-                        0,
-                        0,
-                        currentBytes_,
-                        totalBytes_,
-                        bulkBytesPerSecond_
+                        options_.remotePath, currentIndex_, entries_.size(), 0, 0, currentBytes_, totalBytes_, 0
                     );
                 }
                 Log::info("BulkUploadOperation: Bulk upload completed.");
@@ -368,13 +361,17 @@ std::vector<std::pair<std::filesystem::path, BulkUploadOperation::Error>> BulkUp
 
 void BulkUploadOperation::completeCurrentUpload()
 {
-    currentBytes_ += currentUpload_->totalSize();
+    // Advance by the entry's reported size, not UploadOperation::totalSize()
+    // (the actual on-disk size from tellg). totalBytes_ is summed from the same
+    // reported sizes, so accumulating the reported size keeps currentBytes_
+    // converging exactly to totalBytes_, matching BulkDownloadOperation.
+    currentBytes_ += entries_[currentIndex_].size;
     currentUpload_.reset();
     ++currentIndex_;
     if (!prescannedPathOverride_.empty() && currentIndex_ == entries_.size())
     {
         options_.overallProgressCallback(
-            options_.remotePath, currentIndex_, entries_.size(), 0, 0, currentBytes_, totalBytes_, bulkBytesPerSecond_
+            options_.remotePath, currentIndex_, entries_.size(), 0, 0, currentBytes_, totalBytes_, 0
         );
     }
 }