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.
This commit is contained in:
DrMelone 2026-04-12 23:29:32 +02:00
parent a22aaecd45
commit d68d08b678
2 changed files with 39 additions and 14 deletions

View file

@ -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)

View file

@ -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);
}
};