From d68d08b67857da0e90804f3a65cab269ea4e19f8 Mon Sep 17 00:00:00 2001
From: DrMelone <27028174+Classic298@users.noreply.github.com>
Date: Sun, 12 Apr 2026 23:29:32 +0200
Subject: [PATCH] fix: sync compare syntax, replace scoping, and API error
 surfacing

- Remove stray closing paren in compare_files_for_sync that broke module
  import and prevented the router from loading.
- upload_and_replace_file now verifies the old file belongs to the target
  knowledge base (not just that it exists globally), and propagates a 500
  when the post-upload removal fails so callers can reconcile instead of
  silently accumulating duplicates.
- syncDirectoryHandler splits directory-picker errors (routed through
  handleUploadError) from API errors (now surfaced with the server's
  detail message), so compare/upload/remove failures no longer show the
  misleading "Error accessing directory" toast.
---
 backend/open_webui/routers/knowledge.py       | 21 ++++++++----
 .../workspace/Knowledge/KnowledgeBase.svelte  | 32 ++++++++++++++-----
 2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/backend/open_webui/routers/knowledge.py b/backend/open_webui/routers/knowledge.py
index 977a774261..7b08455084 100644
--- a/backend/open_webui/routers/knowledge.py
+++ b/backend/open_webui/routers/knowledge.py
@@ -737,9 +737,14 @@ async def upload_and_replace_file(
             detail=ERROR_MESSAGES.ACCESS_PROHIBITED,
         )
 
-    # Validate old file exists
+    # Validate old file exists AND belongs to this knowledge base.
+    # Checking only global existence would let callers "replace" a file from
+    # another KB (or no KB) — the new file gets added here while the old one
+    # is untouched or removed from the wrong collection.
     old_file = await Files.get_file_by_id(old_file_id, db=db)
-    if not old_file:
+    if not old_file or not await Knowledges.has_file(
+        knowledge_id=id, file_id=old_file_id, db=db
+    ):
         raise HTTPException(
             status_code=status.HTTP_400_BAD_REQUEST,
             detail=ERROR_MESSAGES.NOT_FOUND,
@@ -794,7 +799,9 @@ async def upload_and_replace_file(
             detail=f'Failed to add file to knowledge base: {str(e)}',
         )
 
-    # Step 4: Remove old file (reuses existing remove_file_from_knowledge_by_id)
+    # Step 4: Remove old file. Any failure here means "replace" semantics
+    # were violated (new file added but old file still present); surface the
+    # error so callers can reconcile instead of silently accumulating dupes.
     try:
         await remove_file_from_knowledge_by_id(
             id=id,
@@ -804,8 +811,11 @@ async def upload_and_replace_file(
             db=db,
         )
     except Exception as e:
-        log.error(f'Failed to remove old file (new file is already added): {e}')
-        # Don't fail - new file is already added successfully
+        log.error(f'Failed to remove old file after replacement upload: {e}')
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail=f'New file uploaded but old file removal failed: {str(e)}',
+        )
 
     return UploadAndReplaceResponse(
         new_file_id=new_file_id,
@@ -1226,7 +1236,6 @@ async def compare_files_for_sync(
                             old_file_id=existing_file.id,
                         )
                     )
-                    )
         else:
             # New file - needs to be uploaded
             new_files.append(incoming_file.file_path)
diff --git a/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte b/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte
index 3207e5adfe..d61f47e6d1 100644
--- a/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte
+++ b/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte
@@ -507,15 +507,26 @@
 
 	// Smart sync: only upload changed files, delete removed files
 	const syncDirectoryHandler = async () => {
+	    // Collect files from the picker first. Errors here are directory/picker
+	    // issues (cancelled, permission denied) — route through handleUploadError.
+	    let directoryFiles;
 	    try {
 	        toast.info($i18n.t('Scanning directory...'));
-	        const directoryFiles = await collectDirectoryFiles({ withHashes: true });
-	
-	        if (directoryFiles.length === 0) {
-	            toast.info($i18n.t('No files found in directory'));
-	            return;
-	        }
-	
+	        directoryFiles = await collectDirectoryFiles({ withHashes: true });
+	    } catch (error) {
+	        handleUploadError(error);
+	        return;
+	    }
+
+	    if (directoryFiles.length === 0) {
+	        toast.info($i18n.t('No files found in directory'));
+	        return;
+	    }
+
+	    // Everything below is API/business logic. Surface the server's detail
+	    // message so users can diagnose compare/upload/remove failures instead
+	    // of seeing a misleading "Error accessing directory" toast.
+	    try {
 	        toast.info(
 	            $i18n.t('Found {{count}} files, comparing with knowledge base...', {
 	                count: directoryFiles.length
@@ -607,7 +618,12 @@
 	        // Refresh the file list
 	        await init();
 	    } catch (error) {
-	        handleUploadError(error);
+	        console.error('Sync error:', error);
+	        const message =
+	            typeof error === 'string'
+	                ? error
+	                : (error?.detail ?? error?.message ?? $i18n.t('Failed to sync directory'));
+	        toast.error(message);
 	    }
 	};