diff --git a/CHANGELOG.md b/CHANGELOG.md index 9412413..f07e367 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -521,6 +521,73 @@ Changes you will notice: nothing else is left to unload. - **Whisper runs on the CPU while the text model is still being loaded** onto a GPU that cannot hold both. +- **A slow move in Organize is no longer reported as failed while it still finishes.** A move to + another drive that took longer than 30 seconds was marked failed, but the file moved anyway, with + no undo entry and stale search paths. The move now runs to its end, and Stop cancels a cross-drive + move before the copy is published. If the drive has stopped answering, Stop ends the batch after + 15 seconds; should the move go through later, the file is put back. +- **An undo that can never work no longer blocks the ones before it.** When the file was deleted or + moved outside the app (or the backup of a deleted file is gone), the entry is removed from history + and the message says so. A file on a drive that is unplugged or ejected keeps its entry. Undo and + redo errors now carry their reason to the screen. +- **Feedback notes no longer raise Organize confidence on their own.** With any note saved, a 69% + match could be shown and auto-organized as 76%, because the ranking score was written into the + confidence. A note now changes confidence only for the folders it is about. +- **A file moved onto a path that an earlier, deleted file used keeps its own search entry** instead + of answering with the old file's content. +- **Organize: smaller fixes.** Clicking an Organize notification focuses the file again; a bulk + category change is saved; "Done! Organized N" and the Complete screen count the files and folders + that were really used; the progress highlight follows a renamed file; counts of one read "1 file", + not "1 files". +- **Smart folders on a network share (`\\server\share\…`) can be organized.** The paths were refused + before they reached the mover. Device paths and administrative shares (`C$`) stay refused. +- **A question, search or description that mentions a Windows path is no longer blanked** before it + is sent (for example "What is in D:\Docs?"). +- **Graph preferences survive a session that expired**, and the "session expired" notice shows on + whichever screen opens first. +- **Several recordings analyzed one by one no longer fail with "busy";** they wait their turn. +- **Model downloads.** A download whose connection closes early fails and resumes instead of + hanging. A vision model whose image projector did not download is reported with a warning, not as + installed. When "Change & rebuild now" fails, Settings goes back to the previous embedding model, + as the app already had. +- **A timed-out image no longer restarts the vision runtime under another image** that is being + analyzed at the same time; images go to the runtime one at a time. +- **Analysis results.** The filename fallback no longer picks a folder because its description + contains the extension's letters inside a word ("ai" in "email"). Changing the organization + instructions re-analyzes files instead of returning the cached answer. Archives that cannot be + listed get keywords, and `.tgz` and `.bz2` count as archives. A document confidence the model gave + as a fraction (0.9) is stored as 90%, not 1%. +- **Smart-folder watcher.** A file the app itself moved out of a watched folder keeps its analysis + history. A file renamed in Explorer keeps its analysis instead of being analyzed again. Turning + the watcher back on while it is still stopping leaves it on. +- **Quitting twice, or closing the window during shutdown, no longer ends the app before cleanup is + done.** +- **"Update ready" stays after a later update check fails.** +- **Search window.** The Timeline list fills the window instead of staying 400 px tall, and the + hidden Atlas tab is no longer reachable with the Tab key. +- **Organizing many or large files to another drive no longer stops after five minutes.** The batch + was cut off, a timeout was shown and the files already moved could be moved back. A batch that is + still moving files now runs until it is done or you press Stop. +- **A failed settings read can no longer replace your saved settings with the defaults.** Settings + now says the read failed and offers Try again; nothing is saved until the real settings have + loaded. +- **Chat.** Dragging files or folders onto the Document scope panel adds them, and says so when + nothing could be added. A reopened chat shows each answer as it first appeared. Retry after a + failed request saves the question with the answer. Chats scoped to more than 200 documents are + saved and keep their memory. +- **Files whose search indexing hit a temporary error are retried on their own** after a short wait, + also after a restart, instead of waiting until another file is analyzed. +- **Analysis history no longer mixes up two files that used the same path.** Analyzing a new file + where an organized file used to be, or moving or deleting a file from there, no longer removes or + re-points the organized file's history. +- **Setup wizard.** No false "Required model downloads stopped" right after clicking Download. When + the AI engine cannot start, the wizard says so with the reason instead of listing installed models + as missing. An image model whose projector file did not download is shown as failed, and the retry + fetches (and Cancel stops) only that file. +- **Dropping a folder whose files are already in the Discover queue no longer analyzes them again;** + only new files are added. +- **Recordings queued behind one that timed out now really wait their turn.** The earlier fix did + not take effect in the packaged app. ## [5.1.2] - 2026-11-01 diff --git a/src/main/analysis/documentAnalysis.js b/src/main/analysis/documentAnalysis.js index 318c877..f50880d 100644 --- a/src/main/analysis/documentAnalysis.js +++ b/src/main/analysis/documentAnalysis.js @@ -48,6 +48,7 @@ const { TEXT_TRUNCATION_MARKER } = require('./documentExtractors'); const { analyzeTextWithLlama, normalizeCategoryToSmartFolders } = require('./documentLlm'); +const { getOrganizationGuidance } = require('./organizationGuidance'); const { getAnalysisLimits } = require('./analysisLimits'); const { normalizeAnalysisResult } = require('./utils'); const { normalizeExtractedTextForStorage } = require('./analysisTextUtils'); @@ -344,12 +345,20 @@ async function analyzeDocumentFile(filePath, smartFolders = [], options = {}) { }); } + // The user's organization instructions shape the answer, so a result cached + // under other instructions must not be returned for this request. + const guidanceSig = crypto + .createHash('sha256') + .update(String((await getOrganizationGuidance()) || '')) + .digest('hex') + .slice(0, 16); + // Step 1: Attempt to compute file signature and check cache (non-fatal if fails) let fileSignature = null; let fileStats = null; try { fileStats = await fs.stat(filePath); - fileSignature = `${ANALYSIS_SIGNATURE_VERSION}|${modelName}|${smartFolderSig}|${filePath}|${fileStats.size}|${fileStats.mtimeMs}`; + fileSignature = `${ANALYSIS_SIGNATURE_VERSION}|${modelName}|${smartFolderSig}|${guidanceSig}|${filePath}|${fileStats.size}|${fileStats.mtimeMs}`; if (!bypassCache) { const cachedResult = getFileCache(fileSignature); if (cachedResult) { @@ -830,9 +839,14 @@ async function analyzeDocumentFile(filePath, smartFolders = [], options = {}) { } else if (SUPPORTED_ARCHIVE_EXTENSIONS.includes(fileExtension)) { // Archive metadata inspection (best-effort) const archiveInfo = await tryExtractArchiveMetadata(filePath); + // An archive with no readable listing has an empty list, not a missing one. + const listedKeywords = Array.isArray(archiveInfo.keywords) + ? archiveInfo.keywords.slice(0, TRUNCATION.KEYWORDS_MAX) + : []; const keywords = - archiveInfo.keywords?.slice(0, TRUNCATION.KEYWORDS_MAX) || - getIntelligentKeywords(fileName, fileExtension); + listedKeywords.length > 0 + ? listedKeywords + : getIntelligentKeywords(fileName, fileExtension); const category = normalizeCategoryToSmartFolders('archive', smartFolders); @@ -957,6 +971,7 @@ async function analyzeDocumentFile(filePath, smartFolders = [], options = {}) { contentHash, fileName, task: 'analyzeTextWithLlama', + guidance: guidanceSig, folders: Array.isArray(smartFolders) ? smartFolders.map((f) => f?.name || '').join(',') : '' diff --git a/src/main/analysis/documentLlm.js b/src/main/analysis/documentLlm.js index d7d38f4..22b3bb5 100644 --- a/src/main/analysis/documentLlm.js +++ b/src/main/analysis/documentLlm.js @@ -364,7 +364,11 @@ async function analyzeTextWithLlama( const namingContextKey = Array.isArray(namingContext) ? namingContext.slice(0, 5).join('|') : ''; - const cacheSeed = `${originalFileName || ''}|${fileDate || ''}|${namingContextKey}|${truncated}`; + // Global user organization instructions (empty string when unset). They go + // into the prompt, so they are part of the key: an answer given under other + // instructions is not an answer to this request. + const orgGuidance = String((await getOrganizationGuidance()) || ''); + const cacheSeed = `${originalFileName || ''}|${fileDate || ''}|${namingContextKey}|${orgGuidance}|${truncated}`; const cacheKey = cacheService.generateKey(cacheSeed, modelToUse, smartFolders); if (!bypassCache) { const cachedResult = cacheService.get(cacheKey); @@ -399,8 +403,7 @@ async function analyzeTextWithLlama( const fileDateContext = fileDate ? `\nDocument File Date: ${fileDate}` : ''; - // Global user organization instructions (empty string when unset). - const orgGuidanceStr = formatOrganizationGuidanceBlock(await getOrganizationGuidance()); + const orgGuidanceStr = formatOrganizationGuidanceBlock(orgGuidance); // Build naming context string if available let namingContextStr = ''; @@ -452,6 +455,7 @@ ${truncated} contentLength: truncated.length, // Additional uniqueness for truncated content text: truncated, model: modelToUse, + guidance: orgGuidance, folders: smartFolders .map((f) => f?.name || '') .filter(Boolean) diff --git a/src/main/analysis/embeddingQueue/EmbeddingQueueCore.js b/src/main/analysis/embeddingQueue/EmbeddingQueueCore.js index 4a4c641..171c626 100644 --- a/src/main/analysis/embeddingQueue/EmbeddingQueueCore.js +++ b/src/main/analysis/embeddingQueue/EmbeddingQueueCore.js @@ -31,6 +31,8 @@ const { processItemsInParallel } = require('./parallelProcessor'); const { createProgressTracker } = require('./progress'); const logger = createLogger('EmbeddingQueue'); +// Shortest wait before the failed-item retry timer fires. +const FAILED_RETRY_MIN_DELAY_MS = 1000; class EmbeddingQueue { /** * @param {{ persistenceFileName?: string, batchSize?: number, flushDelayMs?: number, parallelFlushConcurrency?: number, failedItemsPath?: string, deadLetterPath?: string }} [options] @@ -163,6 +165,11 @@ class EmbeddingQueue { // Track retry timers so they can be cleared on shutdown this._retryTimers = new Set(); + // Wakes the queue when the next failed item's backoff ends (see + // _scheduleFailedRetry). Cleared on shutdown. + /** @type {ReturnType|null} */ + this._failedRetryTimer = null; + // Mutex for flush operation to prevent double-flush race conditions this._flushMutex = Promise.resolve(); @@ -417,9 +424,12 @@ class EmbeddingQueue { // latch if a previous initialize() attempt failed and reset() reran us). this._persistBlocked = false; - if (this.queue.length > 0 || this._failedItemHandler.failedItems.size > 0) { + if (this.queue.length > 0) { this.scheduleFlush(); } + // Restored failed items are re-queued by their own timer: a flush + // with an empty queue returns before it reaches them. + this._scheduleFailedRetry(); } catch (error) { logger.error('[EmbeddingQueue] Initialization error:', error); // The in-memory state is empty/partial while the on-disk state may @@ -467,6 +477,7 @@ class EmbeddingQueue { length: item.vector?.length }); this._failedItemHandler.trackFailedItem(item, 'invalid_vector_format'); + this._scheduleFailedRetry(); return { success: false, reason: 'invalid_vector_format' }; } @@ -499,6 +510,7 @@ class EmbeddingQueue { corruptionPercent: Math.round(corruptionRatio * 100) }); this._failedItemHandler.trackFailedItem(item, 'invalid_vector_values'); + this._scheduleFailedRetry(); return { success: false, reason: 'invalid_vector_values' }; } item.vector = sanitizedVector; @@ -570,6 +582,7 @@ class EmbeddingQueue { // failed-items entry is written by the debounced persist. (This used to // rewrite the whole full queue once per overflowing item.) this._failedItemHandler.trackFailedItem(item, 'queue_overflow'); + this._scheduleFailedRetry(); result.success = false; result.reason = 'queue_overflow'; result.warnings.push('queue_overflow'); @@ -686,6 +699,53 @@ class EmbeddingQueue { if (retryTimer.unref) retryTimer.unref(); } + /** + * Arm a timer for the moment the next failed item's backoff ends, then put + * the due items back on the queue and flush them. Without it a failed item + * was only retried from inside a later flush, and a flush with an empty + * queue returns early — so a failure in the last batch (or a database-offline + * give-up) waited, across restarts, until some other file was enqueued. + * Retry limits and backoff stay with the failed-item handler. + */ + _scheduleFailedRetry() { + if (this._isShuttingDown) return; + if (this._failedRetryTimer) { + clearTimeout(this._failedRetryTimer); + this._failedRetryTimer = null; + } + const delay = this._failedItemHandler.getNextRetryDelay(this.queue); + if (delay === null) return; + + this._failedRetryTimer = setTimeout( + () => { + this._failedRetryTimer = null; + if (this._isShuttingDown) return; + // A running flush re-arms this timer when it ends. + if (this.isFlushing) return; + this._failedItemHandler + .retryFailedItems(this.queue, () => this._schedulePersist({ queue: true })) + .then( + /** @returns {undefined} */ () => { + if (this.queue.length > 0) this.scheduleFlush(); + // Items whose backoff has not ended yet. + this._scheduleFailedRetry(); + return undefined; + } + ) + .catch((/** @type {any} */ err) => + logger.warn('[EmbeddingQueue] Failed-item retry failed:', err?.message) + ); + }, + // The floor keeps a wake-up that re-queues nothing from spinning; the cap + // keeps a bad persisted timestamp from parking the retry for good. + Math.min(Math.max(delay, FAILED_RETRY_MIN_DELAY_MS), RETRY.BACKOFF_MAX_MS) + ); + // Never keep the process alive for a retry; the items are on disk. + if (typeof this._failedRetryTimer.unref === 'function') { + this._failedRetryTimer.unref(); + } + } + /** * Persist queue to disk */ @@ -1020,6 +1080,10 @@ class EmbeddingQueue { // forceFlush's `isFlushing && _activeFlush` read never sees a torn pair. if (resolveActiveFlush) resolveActiveFlush(); this._activeFlush = null; + // Whatever this flush left in the failed-items map (per-item failures, + // an offline give-up) is retried when its backoff ends, even if the + // queue is now empty and nothing else is ever enqueued. + this._scheduleFailedRetry(); } } finally { // Always release the mutex @@ -1560,6 +1624,11 @@ class EmbeddingQueue { this._retryTimers.clear(); } + if (this._failedRetryTimer) { + clearTimeout(this._failedRetryTimer); + this._failedRetryTimer = null; + } + if (this._progressTracker?.clear) { this._progressTracker.clear(); } diff --git a/src/main/analysis/embeddingQueue/failedItemHandler.js b/src/main/analysis/embeddingQueue/failedItemHandler.js index 689a1d2..97a6532 100644 --- a/src/main/analysis/embeddingQueue/failedItemHandler.js +++ b/src/main/analysis/embeddingQueue/failedItemHandler.js @@ -267,6 +267,16 @@ function createFailedItemHandler(config) { markDeadLetterChanged(); } + /** + * Exponential backoff per item: BASE_MS * 2^(retryCount-1), which is + * 5s, 10s, 20s for retryCount 1, 2, 3. + * @param {{ retryCount: number }} data - Failed-items entry + * @returns {number} + */ + function retryBackoffMs(data) { + return RETRY.BACKOFF_BASE_MS * 2 ** (data.retryCount - 1); + } + /** * Get items whose exponential backoff has elapsed. Pure read: `lastAttempt` * is stamped by retryFailedItems only for items it actually re-queues, so an @@ -279,11 +289,7 @@ function createFailedItemHandler(config) { const itemsToRetry = []; for (const data of failedItems.values()) { - // Exponential backoff per item: 10s, 20s, 40s - // Formula: BASE_MS * 2^(retryCount-1) gives 10s, 20s, 40s for retryCount 1, 2, 3 - const backoffMs = RETRY.BACKOFF_BASE_MS * 2 ** (data.retryCount - 1); - - if (now - data.lastAttempt >= backoffMs) { + if (now - data.lastAttempt >= retryBackoffMs(data)) { itemsToRetry.push(data.item); } } @@ -291,6 +297,33 @@ function createFailedItemHandler(config) { return itemsToRetry; } + /** + * How long until the next failed item may be retried, so the owner can arm a + * timer for it instead of waiting for unrelated work to trigger a flush. + * Items already back in the main queue are skipped: their next attempt is + * the queue's own flush. + * @param {any[]} queue - Main queue + * @param {number} [now] + * @returns {number|null} Milliseconds (0 when one is already due), or null when nothing waits + */ + function getNextRetryDelay(queue, now = Date.now()) { + if (failedItems.size === 0) return null; + const queuedIds = new Set(queue.map((item) => item?.id).filter(Boolean)); + /** @type {number|null} */ + let earliest = null; + + for (const data of failedItems.values()) { + const id = data.item?.id; + if (!id || queuedIds.has(id)) continue; + const dueIn = data.lastAttempt + retryBackoffMs(data) - now; + if (!Number.isFinite(dueIn)) continue; + const delay = Math.max(0, dueIn); + if (earliest === null || delay < earliest) earliest = delay; + } + + return earliest; + } + /** * Retry failed items * @param {any[]} queue - Main queue to add retries to @@ -432,6 +465,7 @@ function createFailedItemHandler(config) { trackFailedItem, addToDeadLetterQueue, getItemsToRetry, + getNextRetryDelay, retryFailedItems, getDeadLetterItems, clearDeadLetterQueue, diff --git a/src/main/analysis/fallbackUtils.js b/src/main/analysis/fallbackUtils.js index 50c8018..ef7d071 100644 --- a/src/main/analysis/fallbackUtils.js +++ b/src/main/analysis/fallbackUtils.js @@ -43,6 +43,18 @@ const normalizeCategoryToSmartFolders = return match?.name || uncategorized?.name || folders[0]?.name || 'Uncategorized'; }; +/** + * Whether a folder description names the extension as a word of its own + * (".stl", "stl files"), not as letters inside a longer word ("ai" in "email"). + * @param {string} descLower - Lowercased folder description + * @param {string} extNoDot - Lowercased extension without its dot + * @returns {boolean} + */ +function descriptionListsExtension(descLower, extNoDot) { + const escaped = extNoDot.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp(`(?= 2) { // Match extension with or without dot (e.g., ".stl" or "stl") - if (descLower.includes(lowerExtension) || descLower.includes(extNoDot)) { + if (descriptionListsExtension(descLower, extNoDot)) { score += 15; // High score for explicit extension match in description } } @@ -394,7 +406,9 @@ function getIntelligentCategory(fileName, extension, smartFolders = []) { '.rar': 'Archives', '.7z': 'Archives', '.tar': 'Archives', + '.tgz': 'Archives', '.gz': 'Archives', + '.bz2': 'Archives', '.mp3': 'Music', '.wav': 'Music', '.flac': 'Music', diff --git a/src/main/analysis/imageAnalysis.js b/src/main/analysis/imageAnalysis.js index 52beeb8..2cbda8d 100644 --- a/src/main/analysis/imageAnalysis.js +++ b/src/main/analysis/imageAnalysis.js @@ -452,7 +452,8 @@ async function analyzeImageWithLlama( } // Global user organization instructions (empty string when unset). - const orgGuidanceStr = formatOrganizationGuidanceBlock(await getOrganizationGuidance()); + const orgGuidance = String((await getOrganizationGuidance()) || ''); + const orgGuidanceStr = formatOrganizationGuidanceBlock(orgGuidance); // Build OCR grounding context if text was extracted const ocrGroundingStr = @@ -505,6 +506,7 @@ Rules: fileName: originalFileName, imageHash, model: modelToUse, + guidance: orgGuidance, folders: safeFolders .map((f) => f?.name || '') .filter(Boolean) @@ -1417,7 +1419,14 @@ async function runImageAnalysis(filePath, smartFolders, options) { logger.debug(`Image file size`, { bytes: stats.size }); // Cache quick path: signature based on file stats, before reading bytes. - const signature = `${IMAGE_SIGNATURE_VERSION}|${visionModelName}|${smartFolderSig}|${filePath}|${stats.size}|${stats.mtimeMs}`; + // The user's organization instructions shape the answer, so a result cached + // under other instructions must not be returned for this request. + const guidanceSig = crypto + .createHash('sha256') + .update(String((await getOrganizationGuidance()) || '')) + .digest('hex') + .slice(0, 16); + const signature = `${IMAGE_SIGNATURE_VERSION}|${visionModelName}|${smartFolderSig}|${guidanceSig}|${filePath}|${stats.size}|${stats.mtimeMs}`; if (!bypassCache) { const cached = getImageAnalysisCache().get(signature); if (cached != null) { diff --git a/src/main/analysis/modelAnalysisResult.js b/src/main/analysis/modelAnalysisResult.js index 7c86632..6317141 100644 --- a/src/main/analysis/modelAnalysisResult.js +++ b/src/main/analysis/modelAnalysisResult.js @@ -121,7 +121,12 @@ function buildAnalysisFromModelJson(parsedJson, context) { } parsedJson.confidence = Math.min(95, calculatedConfidence); } else { - parsedJson.confidence = Math.round(rawConfidence); + // The prompt asks for 0-100, but models also answer with a 0-1 fraction. + // Same reading as the image route and normalizeConfidencePercent: a value + // in (0, 1] is a fraction. Rounding it as it stands would leave 0 or 1. + const percentConfidence = + rawConfidence > 0 && rawConfidence <= 1 ? rawConfidence * 100 : rawConfidence; + parsedJson.confidence = Math.round(percentConfidence); } // Build validated result object diff --git a/src/main/core/autoUpdater.js b/src/main/core/autoUpdater.js index 26c7b25..5dc9266 100644 --- a/src/main/core/autoUpdater.js +++ b/src/main/core/autoUpdater.js @@ -362,14 +362,21 @@ function notifyRenderer(payload, getMainWindow) { */ function handleError(err, getMainWindow) { logger.error('[UPDATER] Error:', err); - hasDownloadedUpdate = false; - notifyRenderer( - { - status: 'error', - error: err?.message || 'Update error' - }, - getMainWindow - ); + // A failed check does not discard an installer that already finished + // downloading: electron-updater still installs it on quit. Keep the flag + // (lifecycle reads it at exit) and skip the error status, which would hide + // the renderer's "Update ready" button. + if (hasDownloadedUpdate) { + logger.info('[UPDATER] Keeping the downloaded update pending after the error'); + } else { + notifyRenderer( + { + status: 'error', + error: err?.message || 'Update error' + }, + getMainWindow + ); + } // Surface auto-update failures (network, signature mismatch, manifest // fetch failure, missing latest*.yml) on the DegradedStateBanner so a // user who is not actively looking at the update UI can still tell that @@ -556,8 +563,7 @@ async function initializeAutoUpdater(isDev, getMainWindow) { // the check failed. Previously we just logged, which meant the // UpdateIndicator UI sat in "checking" forever on network // outages / 404s / feed timeouts and the user had no signal the - // check had given up. handleError also resets - // `hasDownloadedUpdate` and emits a structured payload via + // check had given up. handleError emits a structured payload via // IPC_EVENTS.APP_UPDATE that the renderer already understands. handleError(e, getMainWindow); } finally { @@ -644,7 +650,8 @@ async function checkForUpdates() { } try { - hasDownloadedUpdate = false; + // hasDownloadedUpdate is left alone: update-available / update-not-available + // reset it when the check succeeds, and a failed check changes nothing. applyCustomUpdateFeedIfConfigured(); await getAutoUpdater().checkForUpdates(); return { success: true }; diff --git a/src/main/core/lifecycle.js b/src/main/core/lifecycle.js index 02d8b40..96740a4 100644 --- a/src/main/core/lifecycle.js +++ b/src/main/core/lifecycle.js @@ -582,6 +582,10 @@ async function handleBeforeQuit(/** @type {any} */ event) { * Handle window-all-closed event */ function handleWindowAllClosed() { + // A shutdown already running ends in its own app.exit(); the window closing + // meanwhile must not start a second quit. + if (_quitCleanupStarted) return; + // Check if background mode is enabled - if so, don't quit when windows are closed const settingsService = lifecycleConfig.getSettingsService?.(); const backgroundMode = settingsService?.get?.('backgroundMode'); @@ -982,14 +986,21 @@ function registerLifecycleHandlers(createWindow) { // Return cleanup functions for proper teardown return { cleanupAppListeners: () => { - app.removeListener('before-quit', handleBeforeQuit); + // handleBeforeQuit runs this cleanup in its first phase and exits through + // app.exit() only after the last one. The quit gate and the + // window-all-closed handler stay registered until then: without them a + // second Quit, or closing the window (Electron quits by default when + // nothing listens), would end the process while services still persist. + if (!_quitCleanupStarted) { + app.removeListener('before-quit', handleBeforeQuit); + app.removeListener('window-all-closed', handleWindowAllClosed); + } app.removeListener('will-quit', willQuitHandler); if (powerMonitor && typeof powerMonitor.removeListener === 'function') { powerMonitor.removeListener('shutdown', shutdownHandler); } detachLockPowerMonitor(); app.removeListener('quit', quitHandler); - app.removeListener('window-all-closed', handleWindowAllClosed); app.removeListener('activate', activateHandler); }, cleanupProcessListeners: () => { diff --git a/src/main/ipc/files/batchLockManager.js b/src/main/ipc/files/batchLockManager.js index 365d1fc..3528671 100644 --- a/src/main/ipc/files/batchLockManager.js +++ b/src/main/ipc/files/batchLockManager.js @@ -19,11 +19,13 @@ if (typeof createLogger !== 'function' && logger?.setContext) { // Global batch operation lock to prevent concurrent batch operations /** @type {{ batchId: string, acquiredAt: number } | null} */ let batchOperationLock = null; -// Budget for the rollback tail that runs AFTER a batch trips -// MAX_TOTAL_BATCH_TIME: the in-flight move must finish (FILE_COPY) and every -// completed op must be restored. Holders renew the lock per op, so this TTL -// is only the crash backstop — it must still exceed the worst-case -// legitimate hold, or a second batch could start mid-rollback. +// Budget for the rollback tail of a failed batch: every completed op must be +// restored. A batch has no total time limit; MAX_TOTAL_BATCH_TIME only sizes +// this lease. Holders renew the lock per op and on an interval while a move is in flight (a move has +// no time limit of its own; a cross-volume copy of a large file can outlast +// any fixed one), so this TTL is only the crash backstop — it must still +// exceed the worst-case hold between two renewals, or a second batch could +// start mid-rollback. FILE_COPY stays in the sum as slack. const ROLLBACK_ALLOWANCE_MS = 2 * 60 * 1000; const BATCH_LOCK_TIMEOUT = PROCESSING_LIMITS.MAX_TOTAL_BATCH_TIME + TIMEOUTS.FILE_COPY + ROLLBACK_ALLOWANCE_MS; diff --git a/src/main/ipc/files/batchOrganizeHandler.js b/src/main/ipc/files/batchOrganizeHandler.js index 045331b..b83b87e 100644 --- a/src/main/ipc/files/batchOrganizeHandler.js +++ b/src/main/ipc/files/batchOrganizeHandler.js @@ -17,7 +17,7 @@ const { logger: baseLogger, createLogger } = require('../../../shared/logger'); const { crossDeviceMove, renameNoReplace } = require('../../../shared/atomicFileOperations'); const { validateFileOperationPath } = require('../../../shared/pathSanitization'); const { getEffectiveMaxPathLength } = require('../../../shared/securityConfig'); -const { withTimeout, delay, Semaphore } = require('../../../shared/promiseUtils'); +const { withTimeout, delay } = require('../../../shared/promiseUtils'); const { withCorrelationId } = require('../../../shared/correlationId'); const { ERROR_CODES } = require('../../../shared/errorHandlingUtils'); const { deferGhostCleanup, resumeGhostCleanup } = require('../../../shared/ghostCleanupGuard'); @@ -79,7 +79,10 @@ if (typeof createLogger !== 'function' && logger?.setContext) { logger.setContext('IPC:Files:BatchOrganize'); } -// Resource limits from centralized constants (prevents config drift) +// Resource limits from centralized constants (prevents config drift). +// MAX_TOTAL_BATCH_TIME is no longer a limit on a running batch (a batch that +// is still moving files is never cut off for taking long); it is only +// re-exported, and sizes the batch lock's lease in batchLockManager. const MAX_TOTAL_BATCH_TIME = PROCESSING_LIMITS.MAX_TOTAL_BATCH_TIME; const { MAX_NUMERIC_RETRIES } = LIMITS; const FILE_LOCK_ERROR_CODES = new Set(['EBUSY', 'EPERM', 'EACCES']); @@ -89,6 +92,14 @@ const VERIFY_MAX_ATTEMPTS = RETRY?.FILE_OPERATION?.maxAttempts ?? RETRY?.MAX_ATT const VERIFY_BACKOFF_STEP_MS = RETRY?.FILE_OPERATION?.initialDelay ?? TIMEOUTS?.DELAY_TINY ?? MOVE_LOCK_BACKOFF_STEP_MS; const VERIFY_MAX_DELAY_MS = RETRY?.FILE_OPERATION?.maxDelay ?? 5000; +// How often a move still in flight renews the batch lock. A cross-volume move +// of a large file (copy plus a checksum of both sides) has no upper bound, so +// the lease is kept fresh for as long as the move runs. +const MOVE_LOCK_RENEW_INTERVAL_MS = 30 * 1000; +// How long a move that is still in flight gets after the batch is stopped +// before the batch ends without it. +const STUCK_MOVE_ABORT_GRACE_MS = 15 * 1000; +const MOVE_ABANDONED = Symbol('move-abandoned'); const MAX_RESULTS_PER_CHUNK = 100; // Max results per IPC message chunk const YIELD_EVERY_N_OPS = 10; // Yield to event loop every N operations @@ -217,28 +228,39 @@ function _detectPathOverlap(ops) { } /** - * Concurrency limiter built on the shared Semaphore (p-limit pattern). - * Returns a function `limit(fn)` that queues `fn` behind `concurrency` slots. + * Concurrency limiter (p-limit pattern). Returns a function `limit(fn)` that + * queues `fn` behind `concurrency` slots. * - * The queue must be sized for the whole batch: every operation acquires - * up-front, and the Semaphore DEFAULTS (100-entry queue that throws - * QUEUE_FULL, 60s waiter timeout) instantly rejected ops 106+ of a large - * batch — which the unexpected-rejection safety net then escalated into a - * full batch rollback. + * Deliberately not the shared Semaphore: every operation of the batch queues + * up-front, and the Semaphore's queue cap and waiter timeout rejected the + * ones still waiting (ops 106+ of a large batch; every queued op once slow + * cross-volume moves had held the slots for 5 minutes). The + * unexpected-rejection safety net then rolled back every completed move. A + * queued operation here waits for as long as the moves ahead of it take; + * the abort signal is what ends a batch early. * * @param {number} concurrency - Max concurrent tasks - * @param {number} maxQueue - Max waiters (size for the full batch) - * @param {number} queueTimeoutMs - Waiter timeout (align with the batch time budget) * @returns {(fn: () => Promise) => Promise} */ -const pLimit = (concurrency, maxQueue, queueTimeoutMs) => { - const sem = new Semaphore(concurrency, maxQueue, queueTimeoutMs); +const pLimit = (concurrency) => { + let active = 0; + /** @type {Array<() => void>} */ + const waiting = []; return async (fn) => { - await sem.acquire(); + if (active < concurrency) { + active++; + } else { + // The finishing task hands its slot over, so `active` stays as it is. + await new Promise((resolve) => { + waiting.push(() => resolve(undefined)); + }); + } try { return await fn(); } finally { - sem.release(); + const next = waiting.shift(); + if (next) next(); + else active--; } }; }; @@ -488,7 +510,6 @@ async function handleBatchOrganize(params) { // by OrganizeResumeService at next launch. let interruptedCount = 0; // batchId already defined above for lock acquisition - const batchStartTime = Date.now(); let shouldRollback = false; let rollbackReason = null; // Permission failures since the last successful move; a run of them @@ -564,9 +585,7 @@ async function handleBatchOrganize(params) { ); } const batchOperationTotal = batch.operations.length; - // Queue sized for the whole batch; waiter timeout aligned with the - // batch's own time budget (processOperation aborts past it anyway). - const limit = pLimit(effectiveConcurrency, batchOperationTotal + 1, MAX_TOTAL_BATCH_TIME); + const limit = pLimit(effectiveConcurrency); // Abort reasons the USER deliberately triggered (renderer buttons). // Anything else on the signal — 'ipc-timeout', 'shutdown' (abortAll), @@ -609,13 +628,9 @@ async function handleBatchOrganize(params) { } } - if (Date.now() - batchStartTime > MAX_TOTAL_BATCH_TIME) { - log.error(`[FILE-OPS] Batch ${batchId} exceeded maximum time limit`); - shouldRollback = true; - rollbackReason = `Batch exceeded maximum time limit of ${MAX_TOTAL_BATCH_TIME}ms`; - abortController.abort(); - return; - } + // No elapsed-time limit: a batch of large files going to another + // drive takes as long as the copies take. It used to be rolled back + // when an operation started more than 5 minutes in. if (i > 0 && i % YIELD_EVERY_N_OPS === 0) { await new Promise((resolve) => setImmediate(resolve)); @@ -762,16 +777,73 @@ async function handleBatchOrganize(params) { // Handle file move with collision handling // TOCTOU fix: removed verifySourceFile pre-check, handle ENOENT from move directly - // Use timeout for file operations to prevent hangs - const moveWithTimeout = () => - withTimeout( - performFileMove(op, log, computeFileChecksum, abortController.signal), - TIMEOUTS.FILE_COPY, - `File move ${path.basename(op.source)}` - ); + // The move is awaited to its end, never raced against a timer: a + // timer cannot stop a copy, so the row would say "failed" while + // the file went on to move with no undo entry or path update. + // The batch lock is renewed while it runs, and the abort signal + // is what stops a cross-volume move before it publishes. + const moveToCompletion = async () => { + const moveStartedAt = Date.now(); + const lockRenewal = setInterval(() => { + renewBatchLock(batchId); + // File name only (see the skipped-file line below for why). + log.warn('[FILE-OPS] Move still running; the drive is slow or not answering', { + batchId, + index: i, + source: path.basename(op.source), + elapsedMs: Date.now() - moveStartedAt + }); + }, MOVE_LOCK_RENEW_INTERVAL_MS); + lockRenewal.unref?.(); + // A move on a drive that stopped answering can neither finish + // nor be interrupted. Once the batch is stopped (Stop, shutdown, + // rollback) it is given a short grace and then left behind, so + // the batch can end; settleAbandonedMove deals with it if the + // drive answers later. + /** @type {ReturnType | undefined} */ + let graceTimer; + /** @type {(value: typeof MOVE_ABANDONED) => void} */ + let abandon = () => {}; + const abandoned = new Promise((resolve) => { + abandon = resolve; + }); + const startGrace = () => { + graceTimer = setTimeout(() => abandon(MOVE_ABANDONED), STUCK_MOVE_ABORT_GRACE_MS); + graceTimer.unref?.(); + }; + const { signal } = abortController; + if (signal.aborted) startGrace(); + else signal.addEventListener('abort', startGrace, { once: true }); + const movePromise = performFileMove(op, log, computeFileChecksum, signal); + try { + const outcome = await Promise.race([movePromise, abandoned]); + if (outcome !== MOVE_ABANDONED) return outcome; + log.warn( + '[FILE-OPS] Move did not answer after the batch was stopped; left behind', + { + batchId, + index: i, + source: path.basename(op.source), + elapsedMs: Date.now() - moveStartedAt + } + ); + void settleAbandonedMove( + movePromise, + op.source, + () => isUserAbort() || shouldRollback, + log + ); + return { cancelled: true }; + } finally { + clearInterval(lockRenewal); + clearTimeout(graceTimer); + signal.removeEventListener('abort', startGrace); + } + }; + /** @type {any} */ let moveResult; try { - moveResult = await moveWithTimeout(); + moveResult = await moveToCompletion(); } catch (firstMoveError) { let moveError = firstMoveError; // A rename ENOENT is either a missing source or a destination @@ -793,7 +865,7 @@ async function handleBatchOrganize(params) { } try { await fs.mkdir(destDir, { recursive: true }); - moveResult = await moveWithTimeout(); + moveResult = await moveToCompletion(); moveError = null; } catch (retryError) { moveError = retryError; @@ -829,8 +901,9 @@ async function handleBatchOrganize(params) { } if (moveResult && moveResult.cancelled) { - // performFileMove bailed before any rename (user abort landed - // between the gate above and the first rename attempt). + // performFileMove bailed with the source untouched: the abort + // landed between the gate above and the first rename attempt, + // or during a cross-volume copy that was then never published. recordCancelledOp(op); return; } @@ -1651,9 +1724,11 @@ async function performFileMove(op, log, checksumFn, abortSignal = null) { // runs; the no-replace publish then refuses and the walk moves on to // the next suffix, exactly like a same-device EEXIST. try { - await performCrossDeviceMove(op.source, uniqueDestination, log, checksumFn); + await performCrossDeviceMove(op.source, uniqueDestination, log, checksumFn, abortSignal); operationComplete = true; } catch (crossDeviceError) { + // Stopped by the abort before anything was published: nothing moved. + if (isMoveAbortedError(crossDeviceError, abortSignal)) return { cancelled: true }; if (!isNameTakenError(crossDeviceError)) throw crossDeviceError; advanceToNextCandidate(); } @@ -1665,12 +1740,87 @@ async function performFileMove(op, log, checksumFn, abortSignal = null) { // UUID fallback if numeric exhausted if (!operationComplete) { - uniqueDestination = await performUUIDFallback(op, baseName, ext, log, checksumFn); + try { + uniqueDestination = await performUUIDFallback( + op, + baseName, + ext, + log, + checksumFn, + abortSignal + ); + } catch (fallbackError) { + if (isMoveAbortedError(fallbackError, abortSignal)) return { cancelled: true }; + throw fallbackError; + } } return { destination: uniqueDestination }; } +/** + * Follow a move the batch stopped waiting for. The batch reported the file as + * not moved, so when the drive answers after all and the move went through, + * the file is put back where it was. That happens only when shouldReturn() + * says the batch was cancelled or rolled back; an interrupted batch resumes + * from its journal instead. It never rejects. + * @param {Promise} movePromise + * @param {string} source + * @param {() => boolean} shouldReturn + * @param {LoggerLike} log + * @returns {Promise} + */ +async function settleAbandonedMove(movePromise, source, shouldReturn, log) { + let movedTo; + try { + const outcome = await movePromise; + movedTo = outcome && !outcome.cancelled && !outcome.skipped ? outcome.destination : null; + } catch (lateError) { + log.warn('[FILE-OPS] Move left behind by a stopped batch failed; the file did not move', { + source: path.basename(source), + error: /** @type {any} */ (lateError)?.message + }); + return; + } + if (!movedTo) return; + if (!shouldReturn()) { + log.warn('[FILE-OPS] Move left behind by an interrupted batch finished late', { + source: path.basename(source) + }); + return; + } + try { + try { + await renameNoReplace(movedTo, source); + } catch (renameError) { + if (/** @type {any} */ (renameError)?.code !== 'EXDEV') throw renameError; + await crossDeviceMove(movedTo, source, { verify: true }); + } + log.warn('[FILE-OPS] Move finished after the batch was stopped; file put back', { + source: path.basename(source) + }); + } catch (returnError) { + // Full paths on purpose: this file is at a place the app has no record + // of, and the user needs to be able to find it. + log.error('[FILE-OPS] Move finished after the batch was stopped and could not be put back', { + source, + movedTo, + error: /** @type {any} */ (returnError)?.message + }); + } +} + +/** + * Whether a cross-device move stopped because the batch's signal aborted. It + * rejects that way only with the source left in place, so nothing moved. + * @param {any} error + * @param {AbortSignal|null} [abortSignal] + * @returns {boolean} + */ +function isMoveAbortedError(error, abortSignal) { + return Boolean(abortSignal?.aborted) && error?.name === 'AbortError'; +} + /** * Whether a failed move means "that name is taken": a raw EEXIST, or the * FileSystemError(FILE_EXISTS) crossDeviceMove wraps its publish-time EEXIST in. @@ -1729,11 +1879,13 @@ async function renameWithRetry(source, destination, log) { * @param {string} destination * @param {LoggerLike} log * @param {typeof computeFileChecksum} checksumFn + * @param {AbortSignal|null} [abortSignal] - Stops the move before it publishes */ -async function performCrossDeviceMove(source, destination, log, checksumFn) { +async function performCrossDeviceMove(source, destination, log, checksumFn, abortSignal = null) { await crossDeviceMove(source, destination, { verify: true, - checksumFn + checksumFn, + signal: abortSignal }); } @@ -1744,8 +1896,9 @@ async function performCrossDeviceMove(source, destination, log, checksumFn) { * @param {string} ext * @param {LoggerLike} log * @param {typeof computeFileChecksum} checksumFn + * @param {AbortSignal|null} [abortSignal] */ -async function performUUIDFallback(op, baseName, ext, log, checksumFn) { +async function performUUIDFallback(op, baseName, ext, log, checksumFn, abortSignal = null) { log.warn(`[FILE-OPS] Falling back to UUID for ${path.basename(op.source)}`); const maxPathLength = getEffectiveMaxPathLength(); @@ -1771,7 +1924,7 @@ async function performUUIDFallback(op, baseName, ext, log, checksumFn) { if (error.code === 'EEXIST') continue; if (error.code === 'EXDEV') { try { - await performCrossDeviceMove(op.source, uniqueDestination, log, checksumFn); + await performCrossDeviceMove(op.source, uniqueDestination, log, checksumFn, abortSignal); return uniqueDestination; } catch (crossDeviceError) { if (isNameTakenError(crossDeviceError)) continue; diff --git a/src/main/ipc/llama.js b/src/main/ipc/llama.js index 2cde914..2f49ed2 100644 --- a/src/main/ipc/llama.js +++ b/src/main/ipc/llama.js @@ -625,9 +625,16 @@ function registerLlamaIpc(servicesOrParams) { manager .downloadModel(normalizedName, { onProgress }) .then( - /** @type {() => void} */ ( - () => { - logger.info('[IPC:Llama] Model download completed', { model: normalizedName }); + /** @type {(result: any) => void} */ ( + (result) => { + // A vision model whose projector (mmproj) failed to download + // resolves as degraded: the GGUF is installed, but images + // cannot be analyzed. Report it like backgroundSetup does. + const degraded = Boolean(result?.degraded); + logger.info('[IPC:Llama] Model download completed', { + model: normalizedName, + degraded + }); if (win && !win.isDestroyed()) { const latestRequestedProgress = latestProgressByModel.get(normalizedName) || null; @@ -643,7 +650,8 @@ function registerLlamaIpc(servicesOrParams) { safeSend(win.webContents, IPC_EVENTS.OPERATION_PROGRESS, { type: 'model-download-complete', model: normalizedName, - success: true, + success: !degraded, + ...(degraded ? { degraded: true } : {}), progress: { model: normalizedName, filename: normalizedName, @@ -651,7 +659,8 @@ function registerLlamaIpc(servicesOrParams) { downloadedBytes, totalBytes, speedBps: 0, - etaSeconds: 0 + etaSeconds: 0, + ...(degraded ? { companion: result?.companionDownload || null } : {}) } }); } diff --git a/src/main/ipc/validationSchemas.js b/src/main/ipc/validationSchemas.js index 9c9a1ee..3118a90 100644 --- a/src/main/ipc/validationSchemas.js +++ b/src/main/ipc/validationSchemas.js @@ -28,6 +28,7 @@ const { EXPERIENCE_MODE_VALUES } = require('../../shared/experienceMode'); const { collapseDuplicateProtocols } = require('../../shared/urlUtils'); const { logger } = require('../../shared/logger'); const { getEffectiveMaxPathLength } = require('../../shared/securityConfig'); +const { validateFileOperationPathSync } = require('../../shared/pathSanitization'); const { PROVIDER_BASE_URL_RULE, isAllowedProviderBaseUrl @@ -419,6 +420,31 @@ if (!z) { const SAFE_ABSOLUTE_PATH_MESSAGE = 'Path must be absolute and must not be a UNC, URL, or contain ../ traversal segments'; + // Organize / move paths only. Smart folders may be registered on a network + // share, and the handlers behind these schemas accept UNC through + // validateFileOperationPath, with authorizeDestination refusing a UNC + // destination outside a registered Smart Folder. So a \\server\share path + // passes here when that same main-side validation accepts it. Still refused: + // device namespaces (\\?\, \\.\), forward-slash UNC, a server with no share + // and administrative shares (C$, ADMIN$), which would reach system + // directories without matching the dangerous-path list. + /** + * @param {unknown} val + * @returns {boolean} + */ + const isSafeFileOpPath = (val) => { + if (isSafeAbsoluteFsPath(val)) return true; + if (typeof val !== 'string' || process.platform !== 'win32') return false; + const unc = val.match(/^\\\\([^\\/]+)\\([^\\/]+)(?:[\\/]|$)/); + if (!unc) return false; + const [, server, share] = unc; + if (server.startsWith('?') || server.startsWith('.') || share.endsWith('$')) return false; + if (hasPathTraversal(val)) return false; + return validateFileOperationPathSync(val).valid; + }; + const SAFE_FILE_OP_PATH_MESSAGE = + 'Path must be absolute and must not be a URL, a device or administrative share path, or contain ../ traversal segments'; + /** * File path validation - non-empty string with traversal protection */ @@ -746,15 +772,16 @@ if (!z) { */ // IPC-gate path hardening (audit item #1): apply the same safe-absolute-path // refine used by every other path-bearing schema in this file so a 100KB - // string, URL, UNC, or traversal payload fails at the boundary instead of + // string, URL, or traversal payload fails at the boundary instead of // riding all the way to validateFileOperationPath in batchOrganizeHandler.js. + // A network share path is let through on Windows (see isSafeFileOpPath). // Top-level source/destination remain .optional() because move/copy use them // but batch_organize routes through operations[] instead. const fileOpPathSchema = z .string() .min(1) .max(2048) - .refine(isSafeAbsoluteFsPath, SAFE_ABSOLUTE_PATH_MESSAGE); + .refine(isSafeFileOpPath, SAFE_FILE_OP_PATH_MESSAGE); // What the app suggested for this file before the user confirmed. When the // file lands in a different smart folder, batch organize records a diff --git a/src/main/services/ChatHistoryStore.js b/src/main/services/ChatHistoryStore.js index 2f10331..69a00f6 100644 --- a/src/main/services/ChatHistoryStore.js +++ b/src/main/services/ChatHistoryStore.js @@ -45,7 +45,10 @@ const SEARCH_DECRYPT_SCAN_CAP = 500; */ const MAX_CONVERSATION_TITLE = 512; const MAX_MESSAGE_TEXT = 64_000; -const MAX_DOCUMENT_SCOPE_ITEMS = 200; +// Matches chatQuerySchema.documentScopeItems (validationSchemas.js). A lower +// limit here made the first save of an accepted request throw, so the whole +// conversation was never stored. +const MAX_DOCUMENT_SCOPE_ITEMS = 1_000; const MAX_DOCUMENT_SCOPE_ITEM_BYTES = 16_384; const MAX_MESSAGE_SOURCES_ITEMS = 1_000; const MAX_MESSAGE_FOLLOWUPS_ITEMS = 50; diff --git a/src/main/services/DownloadWatcher.js b/src/main/services/DownloadWatcher.js index 32744e2..3c5aacb 100644 --- a/src/main/services/DownloadWatcher.js +++ b/src/main/services/DownloadWatcher.js @@ -129,6 +129,8 @@ class DownloadWatcher { this.downloadsPath = null; this.isStarting = false; this._startPromise = null; // Mutex: concurrent start() callers await the same promise + /** @type {Promise | null} the teardown in flight, awaited by start() */ + this._teardownPromise = null; this.restartAttempts = 0; this.maxRestartAttempts = RETRY.MAX_ATTEMPTS_MEDIUM; this.restartDelay = DEBOUNCE.WATCHER_RESTART_BASE; @@ -202,6 +204,16 @@ class DownloadWatcher { } async start() { + // A teardown keeps this.watcher set until it has drained the downloads in + // flight. Wait for it, or this call reports the dying watcher as running + // and nothing is watched afterwards. + if (this._teardownPromise) { + while (this._teardownPromise) { + await this._teardownPromise.catch(/** @returns {void} */ () => undefined); + } + // That teardown was the app shutting down: stay stopped. + if (this._shutdownRequested) return; + } if (this.watcher) { logger.debug('[DOWNLOAD-WATCHER] Watcher already running'); return; @@ -534,6 +546,19 @@ class DownloadWatcher { * @private */ async _teardown() { + const tearingDown = this._doTeardown(); + this._teardownPromise = tearingDown; + try { + await tearingDown; + } finally { + if (this._teardownPromise === tearingDown) this._teardownPromise = null; + } + } + + /** + * @private + */ + async _doTeardown() { this._stopped = true; // A start() still in flight either sees _stopped and aborts, or lands its diff --git a/src/main/services/ModelDownloadManager.js b/src/main/services/ModelDownloadManager.js index 29e5c28..9dfe217 100644 --- a/src/main/services/ModelDownloadManager.js +++ b/src/main/services/ModelDownloadManager.js @@ -691,6 +691,39 @@ class ModelDownloadManager { finalizeFailure(this._wrapWriteStreamError(error, filename), 'error', true); }); + // A connection that closes cleanly before the body is complete (CDN + // recycle, proxy cut) emits 'aborted'/'close' on the response but no + // 'end' and no request 'error', so pipe() never ends the write stream + // and nothing above settles. Fail as ECONNRESET so the retry path + // resumes from the `.partial` with a Range request. + let prematureCloseHandled = false; + const onPrematureClose = (/** @type {any} */ cause) => { + if (prematureCloseHandled || response.complete || settled || failureClaimed) return; + prematureCloseHandled = true; + const interrupted = /** @type {Error & { code?: string, cause?: any }} */ ( + new Error('Download interrupted - connection closed before the file was complete') + ); + interrupted.code = 'ECONNRESET'; + if (cause instanceof Error) interrupted.cause = cause; + const stream = writeStream; + try { + if (stream) response.unpipe(stream); + } catch { + /* defensive */ + } + // Wait for the file to close before failing: the retry re-stats the + // `.partial` to pick its Range start. + if (!stream || stream.closed) { + finalizeFailure(interrupted); + return; + } + stream.once('close', () => finalizeFailure(interrupted)); + stream.destroy(); + }; + response.on('aborted', onPrematureClose); + response.on('error', onPrematureClose); + response.on('close', onPrematureClose); + // Abort listeners are attached immediately after https.get (below) so // cancel during connect/redirect is honored. Keep stream-event cleanup // here as a belt-and-suspenders release if a settle path skips finalize*. diff --git a/src/main/services/OramaVectorService.js b/src/main/services/OramaVectorService.js index 6ddd619..b12dd0d 100644 --- a/src/main/services/OramaVectorService.js +++ b/src/main/services/OramaVectorService.js @@ -4508,6 +4508,10 @@ class OramaVectorService extends EventEmitter { await this.initialize(); + // Ids this batch still has to move away. A row at one of them belongs to a + // live file (a chain or swap inside one batch), so it is never replaced. + const pendingSourceIds = new Set(pathUpdates.map((spec) => spec?.oldId).filter(Boolean)); + let updated = 0; for (const updateSpec of pathUpdates) { try { @@ -4559,6 +4563,22 @@ class OramaVectorService extends EventEmitter { // If ID changed, insert new then delete old to prevent data loss on insert failure if (newId && newId !== oldId) { + // A row already at the destination id is a leftover: its file was + // deleted or replaced outside the app and the row waits for the + // orphan reaper. Orama's insert throws on an existing id, which left + // the moved file answered by that leftover's vector, so drop the row, + // its sidecar vector and its chunks first. + if (!pendingSourceIds.has(newId) && (await getByID(this._databases.files, newId))) { + await remove(this._databases.files, newId); + this._embeddingStore?.files?.delete(newId); + const staleChunks = await this._removeExactFileChunks(newId, 'update-file-paths'); + if (staleChunks > 0) { + this._emitSearchDataMutated('fileChunks', 'delete', { + fileId: newId, + count: staleChunks + }); + } + } await insert(this._databases.files, { ...existing, ...metaOverrides, @@ -4566,9 +4586,13 @@ class OramaVectorService extends EventEmitter { embedding: safeEmbedding, hasVector, filePath: newPath || existing.filePath, - fileName: newName || existing.fileName + fileName: newName || existing.fileName, + // The file was just seen at its new path. + isOrphaned: false, + orphanedAt: '' }); await remove(this._databases.files, oldId); + pendingSourceIds.delete(oldId); // Migrate the embedding in the sidecar store. Zero-vector placeholders are // intentionally left uncached so the sidecar-repair pass can detect and diff --git a/src/main/services/SmartFolderWatcher.js b/src/main/services/SmartFolderWatcher.js index 4f6c040..7ac3956 100644 --- a/src/main/services/SmartFolderWatcher.js +++ b/src/main/services/SmartFolderWatcher.js @@ -54,6 +54,7 @@ const { } = require('../../shared/performanceConstants'); const { findUserExclusion } = require('./watcherExclusions'); const { getInstance: getFileOperationTracker } = require('../../shared/fileOperationTracker'); +const { isGhostCleanupDeferred } = require('../../shared/ghostCleanupGuard'); const { normalizePathForIndex, getCanonicalFileId, @@ -198,7 +199,10 @@ const QUEUE_DROP_LOG_INTERVAL_MS = 10_000; const QUEUE_PERSIST_DEBOUNCE_MS = 1000; const QUEUE_PERSIST_DEBOUNCE_LARGE_MS = 10_000; const QUEUE_PERSIST_LARGE_THRESHOLD = 2000; -const MOVE_DETECTION_WINDOW_MS = 3000; +// How long a deletion waits for its matching `add` beyond the time that add +// needs to arrive: chokidar holds it for stabilityThreshold (checked on a +// 200 ms poll) and _handleFileEvent then debounces it. +const MOVE_DETECTION_MARGIN_MS = 2000; const MOVE_MTIME_TOLERANCE_MS = 2000; // File extensions that have dedicated parsers/AI analysis paths. @@ -326,6 +330,8 @@ class SmartFolderWatcher { // stop() has returned and cleared _isStopping (its in-flight wait is // bounded, and an aborted analysis may unwind later than that). this._stopEpoch = 0; + /** @type {Promise | null} the stop() in flight, awaited by start() */ + this._stopPromise = null; // Restart machinery for chokidar 'error' recovery. Without this the // previous behavior was: log + bump errors counter, isRunning stays @@ -407,7 +413,11 @@ class SmartFolderWatcher { const envDepth = parseInt(process.env.STRATOSORT_WATCHER_DEPTH, 10); this.watcherDepth = Number.isFinite(envDepth) && envDepth >= 0 && envDepth <= 32 ? envDepth : 10; - this._moveDetectionWindowMs = MOVE_DETECTION_WINDOW_MS; + // Must outlast stabilityThreshold + debounceDelay: with a shorter window + // the deletion was finalized before the renamed file's add was looked at, + // so every rename in Explorer was a delete plus a full re-analysis. + this._moveDetectionWindowMs = + this.stabilityThreshold + this.debounceDelay + MOVE_DETECTION_MARGIN_MS; this._moveMtimeToleranceMs = MOVE_MTIME_TOLERANCE_MS; // Sync concurrency from settings (non-blocking) @@ -469,7 +479,9 @@ class SmartFolderWatcher { // graceful shutdown to await. const wantsDisabled = settings.smartFolderWatchEnabled === false; const shouldStop = wantsDisabled && (this.isRunning || this.isStarting); - const shouldStart = !wantsDisabled && !this.isRunning && !this.isStarting; + // A watcher that is still stopping counts as stopped: isRunning stays true + // until stop() has closed it, and start() waits for that stop. + const shouldStart = !wantsDisabled && !this.isStarting && (!this.isRunning || this._isStopping); if (shouldStop || shouldStart) { const toggle = shouldStop ? this.stop() : this.start(); this._watchToggleWork = Promise.resolve(toggle).catch((error) => { @@ -581,6 +593,16 @@ class SmartFolderWatcher { * @returns {Promise} True if started successfully */ async start() { + // stop() leaves isRunning true until it has closed the watcher. Wait for + // it, or this call reports the dying watcher as running and nothing is + // watched afterwards. + if (this._stopPromise) { + while (this._stopPromise) { + await this._stopPromise.catch(() => {}); + } + // That stop was the app shutting down: stay stopped. + if (this._shutdownRequested) return false; + } if (this.isRunning) { logger.debug('[SMART-FOLDER-WATCHER] Already running'); return true; @@ -878,6 +900,20 @@ class SmartFolderWatcher { * FIX MED-6: Clean up pending move detection timeouts to prevent callbacks after stop */ async stop() { + const stopping = this._doStop(); + this._stopPromise = stopping; + try { + await stopping; + } finally { + if (this._stopPromise === stopping) this._stopPromise = null; + } + } + + /** + * Internal stop implementation + * @private + */ + async _doStop() { logger.info('[SMART-FOLDER-WATCHER] Stopping...'); this._isStopping = true; this._stopEpoch++; @@ -3800,6 +3836,25 @@ class SmartFolderWatcher { return; } + // An unlink is also what a move out of the folder by the app looks like + // (organize, undo/redo). The file still exists at its new path, and its + // history entry still matches this one through originalPath, so cleanup + // here would delete the analysis of a live file. + if (isGhostCleanupDeferred()) { + logger.debug( + '[SMART-FOLDER-WATCHER] Skipping deletion cleanup: batch organize in progress', + filePath + ); + return; + } + if (getFileOperationTracker().wasRecentlyOperated(filePath)) { + logger.debug( + '[SMART-FOLDER-WATCHER] Skipping deletion cleanup: recently moved by app', + filePath + ); + return; + } + logger.info('[SMART-FOLDER-WATCHER] Detected external file deletion:', filePath); try { diff --git a/src/main/services/UndoRedoService.js b/src/main/services/UndoRedoService.js index 1e7f14b..032d00c 100644 --- a/src/main/services/UndoRedoService.js +++ b/src/main/services/UndoRedoService.js @@ -191,12 +191,13 @@ class UndoRedoService { ? normalizedSource.toLowerCase() === normalizedDestination.toLowerCase() : normalizedSource === normalizedDestination; if (!(await this.fileExists(normalizedSource))) { - const missingErr = /** @type {Error & { code?: string }} */ ( + const missingErr = /** @type {Error & { code?: string, path?: string }} */ ( new Error( `Cannot move: source no longer exists (moved or deleted outside the app): ${normalizedSource}` ) ); missingErr.code = 'UNDO_SOURCE_MISSING'; + missingErr.path = normalizedSource; throw missingErr; } if (!isSameFileTarget && (await this.fileExists(normalizedDestination))) { @@ -1179,8 +1180,122 @@ class UndoRedoService { logger.error('[UndoRedoService] Failed to undo action', { error: error.message }); - throw new Error(`Failed to undo action: ${error.message}`, { cause: error }); + // The batch branch above always advances; a single action that can + // never be reversed must not hold the cursor either, or every Undo + // retries it and nothing older can be undone. + if (await this._isPermanentlyIrreversible(error)) { + return this._dropIrreversibleAction(action, 'undo', error); + } + // Keep the code on the wrapper: the IPC layer forwards error.code so + // the renderer can tell the user what to do. + const wrapped = /** @type {Error & { code?: string }} */ ( + new Error(`Failed to undo action: ${error.message}`, { cause: error }) + ); + if (error?.code) wrapped.code = error.code; + throw wrapped; + } + } + + /** + * True when a missing file is gone rather than out of reach: the path + * itself reports ENOENT while its volume root still answers. An unplugged + * or offline drive fails the root probe. + * @private + * @param {any} filePath + * @returns {Promise} + */ + async _isGoneForGood(filePath) { + if (typeof filePath !== 'string' || !filePath) return false; + try { + await fs.stat(filePath); + return false; + } catch (statError) { + if (/** @type {any} */ (statError)?.code !== 'ENOENT') return false; + } + // On macOS and Linux the root is always "/", so probe the mount point of + // a removable or network volume instead: an ejected drive leaves no + // /Volumes/, and its files must not read as deleted. + const mount = filePath.match( + /^(\/Volumes\/[^/]+|\/media\/[^/]+\/[^/]+|\/run\/media\/[^/]+\/[^/]+|\/mnt\/[^/]+)\// + ); + try { + await fs.access(mount ? mount[1] : path.parse(filePath).root); + return true; + } catch { + return false; + } + } + + /** + * Whether a failed single undo/redo can never succeed on a retry: the file + * (or the delete backup) it needs no longer exists. An occupied destination + * or a changed copy is something the user can fix, and a file on a volume + * that is not reachable right now may come back; those keep their entry. + * @private + * @param {any} error + * @returns {Promise} + */ + async _isPermanentlyIrreversible(error) { + const code = error?.code; + if (code !== 'UNDO_SOURCE_MISSING' && code !== 'UNDO_BACKUP_MISSING') return false; + // A delete recorded without any backup has nothing to wait for. + if (code === 'UNDO_BACKUP_MISSING' && !error.path) return true; + return this._isGoneForGood(error.path); + } + + /** + * Remove an entry that can never be undone or redone, as the trash-only + * branch of _doUndo does, and say so: the result names the entry, the + * reason and the fact that it left history, so the failure is never silent. + * @private + * @param {any} action + * @param {'undo'|'redo'} direction + * @param {any} error + */ + async _dropIrreversibleAction(action, direction, error) { + const index = this.actions.indexOf(action); + if (index !== -1) { + this.actions.splice(index, 1); + if (index <= this.currentIndex) this.currentIndex--; + this._recalculateMemoryEstimate(); } + try { + await this._saveActionsNow(); + } catch (saveError) { + logger.warn('[UndoRedoService] Could not persist history after dropping an entry', { + actionId: action.id, + error: /** @type {any} */ (saveError)?.message + }); + } + logger.warn( + `[UndoRedoService] Entry can never be ${direction === 'undo' ? 'undone' : 'redone'}; removed from history`, + { + actionId: action.id, + type: action.type, + description: action.description, + code: error.code, + error: error.message + } + ); + this._notifyStateChanged('remove'); + const reason = + error.code === 'UNDO_BACKUP_MISSING' + ? 'the backup of the deleted file is gone' + : 'the file was moved or deleted outside the app'; + return { + success: false, + code: error.code, + removedFromHistory: true, + action: { + id: action.id, + type: action.type, + description: action.description, + timestamp: action.timestamp + }, + message: `Cannot ${direction} "${action.description}": ${reason}. The entry was removed from history.`, + error: error.message, + pathMutations: /** @type {Array<{ oldPath: string, newPath: string }>} */ ([]) + }; } /** @@ -1319,7 +1434,16 @@ class UndoRedoService { logger.error('[UndoRedoService] Failed to redo action', { error: error.message }); - throw new Error(`Failed to redo action: ${error.message}`, { cause: error }); + // Same as undo: an entry whose file is gone leaves history, and the + // wrapper keeps the code. + if (await this._isPermanentlyIrreversible(error)) { + return this._dropIrreversibleAction(action, 'redo', error); + } + const wrapped = /** @type {Error & { code?: string }} */ ( + new Error(`Failed to redo action: ${error.message}`, { cause: error }) + ); + if (error?.code) wrapped.code = error.code; + throw wrapped; } } @@ -1421,11 +1545,15 @@ class UndoRedoService { // Previous code didn't persist backup paths immediately, risking data loss on crashes // Now we check multiple backup locations and provide detailed error information if (!action.data.backupPath) { - throw new Error( - 'Cannot restore deleted file - no backup path was recorded. ' + - 'File may have been permanently deleted without backup. ' + - `Original path: ${action.data.originalPath}` + const noBackupErr = /** @type {Error & { code?: string }} */ ( + new Error( + 'Cannot restore deleted file - no backup path was recorded. ' + + 'File may have been permanently deleted without backup. ' + + `Original path: ${action.data.originalPath}` + ) ); + noBackupErr.code = 'UNDO_BACKUP_MISSING'; + throw noBackupErr; } const backupExists = await this.fileExists(action.data.backupPath); @@ -1439,13 +1567,18 @@ class UndoRedoService { timestamp: action.timestamp }); - throw new Error( - `Cannot restore deleted file - backup not found at expected location.\n` + - `Original file: ${action.data.originalPath}\n` + - `Expected backup: ${action.data.backupPath}\n` + - `This may indicate the backup was never created, was deleted, or the path is incorrect.\n` + - `Action ID: ${action.id}, Timestamp: ${action.timestamp}` + const backupErr = /** @type {Error & { code?: string, path?: string }} */ ( + new Error( + `Cannot restore deleted file - backup not found at expected location.\n` + + `Original file: ${action.data.originalPath}\n` + + `Expected backup: ${action.data.backupPath}\n` + + `This may indicate the backup was never created, was deleted, or the path is incorrect.\n` + + `Action ID: ${action.id}, Timestamp: ${action.timestamp}` + ) ); + backupErr.code = 'UNDO_BACKUP_MISSING'; + backupErr.path = action.data.backupPath; + throw backupErr; } // Restore file from backup @@ -1566,10 +1699,11 @@ class UndoRedoService { const copySrc = await this._validateActionPath(action.data.originalPath, 'copy-source'); const copyDest = await this._validateActionPath(action.data.newPath, 'copy-destination'); if (!(await this.fileExists(copySrc))) { - const missingErr = /** @type {Error & { code?: string }} */ ( + const missingErr = /** @type {Error & { code?: string, path?: string }} */ ( new Error(`Cannot redo copy: source no longer exists: ${copySrc}`) ); missingErr.code = 'UNDO_SOURCE_MISSING'; + missingErr.path = copySrc; throw missingErr; } try { diff --git a/src/main/services/VisionService.js b/src/main/services/VisionService.js index 8ef48e9..9091a41 100644 --- a/src/main/services/VisionService.js +++ b/src/main/services/VisionService.js @@ -1055,6 +1055,10 @@ class VisionService { * @type {Set<{ manager: any }>} */ this._requestHolds = new Set(); + // Settles when the last request that asked for a turn is over. See + // _waitForRequestTurn. + /** @type {Promise} */ + this._requestTurn = Promise.resolve(); /** @type {{ manager: any, type: string } | null} */ this._memoryRegistration = null; /** @type {any} */ @@ -1205,6 +1209,53 @@ class VisionService { }; } + /** + * Wait until the requests ahead of this one are over. + * + * The runtime is started with `--parallel 1`, while the vision inference + * slot can admit two or three images on a large GPU. A second request sent + * anyway waited inside the runtime with its timeout already running, and + * the restart after one request's timeout cut the connection of the other, + * which failed without a retry. One request at a time here means a restart + * never has another request on the runtime. + * + * A caller that gives up while it waits leaves at once and never reaches + * the runtime. + * @param {AbortSignal} [signal] + * @returns {Promise<() => void>} ends the turn + */ + async _waitForRequestTurn(signal) { + const ahead = this._requestTurn; + /** @type {() => void} */ + let endTurn = () => {}; + /** @type {Promise} */ + const mine = new Promise((resolve) => { + endTurn = resolve; + }); + this._requestTurn = ahead.then(() => mine); + if (!signal) { + await ahead; + return endTurn; + } + /** @type {() => void} */ + let onAbort = () => {}; + try { + await Promise.race([ + ahead, + new Promise((_resolve, reject) => { + onAbort = () => reject(createVisionAbortError()); + signal.addEventListener('abort', onAbort, { once: true }); + }) + ]); + } catch (error) { + endTurn(); + throw error; + } finally { + signal.removeEventListener('abort', onAbort); + } + return endTurn; + } + _extractDecodePressureHint() { const recent = String(this._recentStderr || ''); if (!recent) return ''; @@ -2054,7 +2105,10 @@ class VisionService { // read) in which the runtime was taken from under the image that had // started it, and the image fell back to OCR. const endRequest = this._beginRequest(); + /** @type {(() => void) | null} */ + let endTurn = null; try { + endTurn = await this._waitForRequestTurn(signal); return await this._analyzeImageWithRuntime({ imageBase64, imagePath, @@ -2067,6 +2121,7 @@ class VisionService { config }); } finally { + endTurn?.(); endRequest(); // Every way out re-arms the idle timer cleared above. A failure between // the runtime starting and the request going out (image file gone, a diff --git a/src/main/services/analysisHistory/AnalysisHistoryServiceCore.js b/src/main/services/analysisHistory/AnalysisHistoryServiceCore.js index 21561fd..bf708a6 100644 --- a/src/main/services/analysisHistory/AnalysisHistoryServiceCore.js +++ b/src/main/services/analysisHistory/AnalysisHistoryServiceCore.js @@ -74,6 +74,18 @@ const { const { performMaintenanceIfNeeded, migrateHistory } = require('./maintenance'); const logger = createLogger('AnalysisHistoryService'); + +/** + * Where an entry's file lives now: `organization.actual` once it has been + * moved, `originalPath` until then. `originalPath` is never rewritten, so + * after a move it names a place another file may since have taken. Path-keyed + * writes match on this, never on a path the file has left. + * @param {any} entry + * @returns {string|null} + */ +function currentPathOf(entry) { + return entry?.organization?.actual || entry?.originalPath || null; +} class AnalysisHistoryServiceCore { constructor() { this.userDataPath = app.getPath('userData'); @@ -1286,12 +1298,11 @@ class AnalysisHistoryServiceCore { const entries = this.analysisHistory?.entries || {}; for (const entry of Object.values(entries)) { const actualPath = entry.organization?.actual || null; - const updateKey = - entry.originalPath && updateMap.has(entry.originalPath) - ? entry.originalPath - : actualPath && updateMap.has(actualPath) - ? actualPath - : null; + // Current path only. A moved entry's originalPath may be another + // file's path now; matching it re-pointed this entry at that file's + // destination and left the moved file without history. + const currentPath = currentPathOf(entry); + const updateKey = currentPath && updateMap.has(currentPath) ? currentPath : null; const update = updateKey ? updateMap.get(updateKey) : null; if (update) { // Track old actual path for index update @@ -1442,7 +1453,7 @@ class AnalysisHistoryServiceCore { } /** - * Update tags for an entry identified by path. + * Update tags for an entry identified by its current path. * * @param {string} filePath * @param {string[]} tags @@ -1464,9 +1475,7 @@ class AnalysisHistoryServiceCore { const now = new Date().toISOString(); for (const entry of Object.values(entries)) { - const original = entry?.originalPath; - const actual = entry?.organization?.actual; - if (original !== target && actual !== target) continue; + if (currentPathOf(entry) !== target) continue; // Clone for index removal const oldEntry = deepClone(entry); @@ -1506,7 +1515,8 @@ class AnalysisHistoryServiceCore { * * This is the persistence layer for staged embedding workflows. * - * Matches either the original path or the organization.actual path. + * Matches the entry's current path (organization.actual once moved, + * otherwise the original path). * * @param {string} filePath * @param {{policy?: 'embed'|'skip'|'web_only'|null, status?: 'unknown'|'pending'|'done'|'skipped'|'error'|null, model?: string|null, error?: string|null}} update @@ -1542,9 +1552,7 @@ class AnalysisHistoryServiceCore { }; for (const entry of Object.values(entries)) { - const original = entry?.originalPath; - const actual = entry?.organization?.actual; - if (original !== target && actual !== target) continue; + if (currentPathOf(entry) !== target) continue; if (!entry.embedding || typeof entry.embedding !== 'object') { entry.embedding = { policy: null, status: 'unknown', model: null, updatedAt: now }; @@ -1596,7 +1604,10 @@ class AnalysisHistoryServiceCore { * Remove analysis history entries associated with a path. * Used to keep BM25/analysis-history-backed search in sync after deletes. * - * Matches either the original path or the organization.actual path. + * Matches the entry's current path (organization.actual once moved, + * otherwise the original path). A path the file has left never matches: a + * different file may live there now, and deleting it must not take the moved + * file's history with it. * * @param {string} filePath * @returns {Promise<{removed: number, error?: string}>} @@ -1614,9 +1625,7 @@ class AnalysisHistoryServiceCore { try { const entries = this.analysisHistory?.entries || {}; for (const [id, entry] of Object.entries(entries)) { - const original = entry?.originalPath; - const actual = entry?.organization?.actual; - if (original === target || actual === target) { + if (currentPathOf(entry) === target) { removedEntries.push({ id, fileHash: entry.fileHash, diff --git a/src/main/services/analysisHistory/indexManager.js b/src/main/services/analysisHistory/indexManager.js index 1140600..37d3e40 100644 --- a/src/main/services/analysisHistory/indexManager.js +++ b/src/main/services/analysisHistory/indexManager.js @@ -57,6 +57,51 @@ function getSizeRange(size) { return 'huge'; // >= 100MB } +/** + * Where the entry's file lives now: `organization.actual` once it has been + * moved, `originalPath` until then. `originalPath` is never rewritten, so after + * a move it names a place another file may since have taken. + * @param {Record} entry - Analysis entry + * @returns {string|null} Current path, or null when the entry has none + */ +function getEntryCurrentPath(entry) { + const actual = entry?.organization?.actual; + if (typeof actual === 'string' && actual) return actual; + const original = entry?.originalPath; + return typeof original === 'string' && original ? original : null; +} + +/** + * Whether the entry's file currently lives at `filePath` (exact or + * index-normalized match). + * @param {Record} entry - Analysis entry + * @param {string} filePath - Path to compare + * @returns {boolean} + */ +function entryLivesAtPath(entry, filePath) { + const current = getEntryCurrentPath(entry); + if (!current || typeof filePath !== 'string' || !filePath) return false; + if (current === filePath) return true; + const normalizedCurrent = normalizePathForIndex(current); + return Boolean(normalizedCurrent) && normalizedCurrent === normalizePathForIndex(filePath); +} + +/** + * Drop a path's lookup keys, but only the ones that still point at this entry: + * a newer entry for another file may own the same path by now. + * @param {Record} index - Analysis index + * @param {string} filePath - Path whose keys to drop + * @param {string} entryId - Entry the keys must belong to + */ +function deletePathKeysOwnedBy(index, filePath, entryId) { + if (!filePath) return; + if (index.pathLookup[filePath] === entryId) delete index.pathLookup[filePath]; + const normalized = normalizePathForIndex(filePath); + if (normalized && normalized !== filePath && index.pathLookup[normalized] === entryId) { + delete index.pathLookup[normalized]; + } +} + /** * Update all indexes for a new entry * @param {Record} index - Analysis index @@ -71,11 +116,15 @@ function updateIndexes(index, entry) { // File hash index index.fileHashes[entry.fileHash] = entry.id; - // Path lookup index (store both original and normalized for case-insensitive FS) - index.pathLookup[entry.originalPath] = entry.id; - const normalizedPath = normalizePathForIndex(entry.originalPath); - if (normalizedPath && normalizedPath !== entry.originalPath) { - index.pathLookup[normalizedPath] = entry.id; + // Path lookup index (store both original and normalized for case-insensitive FS). + // A moved entry is indexed under its current path only: its originalPath may + // belong to another file now, and must not shadow that file's entry. + if (!entry.organization?.actual || entryLivesAtPath(entry, entry.originalPath)) { + index.pathLookup[entry.originalPath] = entry.id; + const normalizedPath = normalizePathForIndex(entry.originalPath); + if (normalizedPath && normalizedPath !== entry.originalPath) { + index.pathLookup[normalizedPath] = entry.id; + } } if (entry.organization?.actual) { @@ -127,19 +176,13 @@ function updateIndexes(index, entry) { function removeFromIndexes(index, entry) { // Remove from various indexes delete index.fileHashes[entry.fileHash]; - const normalizedPath = normalizePathForIndex(entry.originalPath); - delete index.pathLookup[entry.originalPath]; - if (normalizedPath && normalizedPath !== entry.originalPath) { - delete index.pathLookup[normalizedPath]; - } + // Path keys go only while they still point at this entry: the path a moved + // entry left may be another file's key by now. + deletePathKeysOwnedBy(index, entry.originalPath, entry.id); // Also remove organization.actual paths to prevent orphaned index entries if (entry.organization?.actual) { - delete index.pathLookup[entry.organization.actual]; - const normalizedActual = normalizePathForIndex(entry.organization.actual); - if (normalizedActual && normalizedActual !== entry.organization.actual) { - delete index.pathLookup[normalizedActual]; - } + deletePathKeysOwnedBy(index, entry.organization.actual, entry.id); } // Remove from tag index @@ -193,7 +236,7 @@ function removeFromIndexes(index, entry) { * * @param {Record} index - Analysis index * @param {Record} entry - Entry that was updated - * @param {string} oldActualPath - Previous organization.actual path (if any) + * @param {string|null} oldActualPath - Previous organization.actual path (if any) * @param {string} newActualPath - New organization.actual path */ function updatePathIndexForMove(index, entry, oldActualPath, newActualPath) { @@ -201,11 +244,13 @@ function updatePathIndexForMove(index, entry, oldActualPath, newActualPath) { // Remove old actual path from index (if any) if (oldActualPath) { - delete index.pathLookup[oldActualPath]; - const normalizedOld = normalizePathForIndex(oldActualPath); - if (normalizedOld && normalizedOld !== oldActualPath) { - delete index.pathLookup[normalizedOld]; - } + deletePathKeysOwnedBy(index, oldActualPath, entry.id); + } + + // The file has left originalPath too (a first move has no old actual path). + // The keys are re-added below when the move brings it back there. + if (entry.originalPath) { + deletePathKeysOwnedBy(index, entry.originalPath, entry.id); } // Add new actual path to index diff --git a/src/main/services/analysisHistory/queries.js b/src/main/services/analysisHistory/queries.js index 50ef295..a8bb762 100644 --- a/src/main/services/analysisHistory/queries.js +++ b/src/main/services/analysisHistory/queries.js @@ -59,7 +59,27 @@ function sortEntries(entries, sortBy, sortOrder) { } /** - * Get analysis by file path + * Whether a path-lookup hit is the entry for the file that lives at the path + * now. An index saved before moved entries dropped their originalPath key can + * still map the path a file left to that file's entry, and a different file + * may sit there today. A key whose entry is gone, or an entry without any + * path (legacy/partial), is left for the caller to resolve as before. + * @param {Record|undefined} entry - Candidate entry + * @param {string} filePath - Path as asked + * @param {string} normalized - Normalized form of `filePath` + * @returns {boolean} + */ +function entryIsAtPath(entry, filePath, normalized) { + if (!entry) return true; + const current = entry.organization?.actual || entry.originalPath; + if (typeof current !== 'string' || !current) return true; + return current === filePath || normalizePathForLookup(current) === normalized; +} + +/** + * Get analysis by file path. Resolves the entry whose file lives at the path + * now (`organization.actual`, or `originalPath` when it was never moved), not + * an entry that merely started out there. * @param {Record} analysisHistory - Analysis history data * @param {Record} analysisIndex - Analysis index * @param {string} filePath - File path to look up @@ -71,14 +91,22 @@ function getAnalysisByPath(analysisHistory, analysisIndex, filePath) { const normalized = normalizePathForLookup(filePath); // Fast paths: exact key, then normalized key (for new indexes) - let entryId = - analysisIndex.pathLookup[filePath] ?? - (normalized ? analysisIndex.pathLookup[normalized] : undefined); + let entryId; + for (const key of [filePath, normalized]) { + const candidateId = key ? analysisIndex.pathLookup[key] : undefined; + if (candidateId && entryIsAtPath(analysisHistory.entries[candidateId], filePath, normalized)) { + entryId = candidateId; + break; + } + } // Backfill for legacy indexes that only stored the original-cased path on Windows if (!entryId && normalized && process.platform === 'win32') { for (const [storedPath, id] of Object.entries(analysisIndex.pathLookup)) { - if (normalizePathForLookup(storedPath) === normalized) { + if ( + normalizePathForLookup(storedPath) === normalized && + entryIsAtPath(analysisHistory.entries[id], filePath, normalized) + ) { entryId = id; // Cache the normalized key to avoid future scans analysisIndex.pathLookup[normalized] = id; diff --git a/src/main/services/chat/SessionMemory.js b/src/main/services/chat/SessionMemory.js index 19ef852..5414369 100644 --- a/src/main/services/chat/SessionMemory.js +++ b/src/main/services/chat/SessionMemory.js @@ -128,7 +128,9 @@ class SessionMemory { if (this.chatHistoryStore && memory?.sessionId) { try { const conv = this.chatHistoryStore.getConversation(memory.sessionId); - if (!conv) return ''; + // No stored conversation: a turn that could not be persisted lives in + // the in-memory buffer, so follow-ups still read their history there. + if (!conv) return this._getInMemoryHistoryText(memory); const messages = conv.messages.slice(-(this.memoryWindow * 2)); return messages .map((/** @type {any} */ m) => { @@ -169,6 +171,11 @@ class SessionMemory { } } + return this._getInMemoryHistoryText(memory); + } + + /** @private */ + async _getInMemoryHistoryText(/** @type {any} */ memory) { try { const vars = await memory.loadMemoryVariables({}); return typeof vars?.history === 'string' ? vars.history : ''; @@ -208,10 +215,13 @@ class SessionMemory { // Persist user turn — unless this is a regenerate, where the original // user row already exists and re-inserting duplicates it on reload. - if (!skipUserMessage) { + // Retry after a failed request sends the same flag although nothing + // was saved, so the skip is honoured only when the row is really there. + const userText = typeof input === 'string' ? input : ''; + if (!skipUserMessage || !this._isLastUserMessage(conv, userText)) { this.chatHistoryStore.addMessage(memory.sessionId, { role: 'user', - text: typeof input === 'string' ? input : '' + text: userText }); } @@ -260,6 +270,22 @@ class SessionMemory { } } + /** + * Whether the conversation's most recent user row is this question. + * @private + * @param {any} conv Stored conversation, or null when none exists yet. + * @param {string} userText + * @returns {boolean} + */ + _isLastUserMessage(conv, userText) { + const messages = Array.isArray(conv?.messages) ? conv.messages : []; + for (let i = messages.length - 1; i >= 0; i -= 1) { + if (messages[i]?.role !== 'user') continue; + return String(messages[i].text ?? '').trim() === userText.trim(); + } + return false; + } + /** * Remap per-turn `doc-N` ids to conversation-scoped FIRST-SEEN ordinals so * the same document keeps its number across turns. Pre-fix, the retrieval diff --git a/src/main/services/organization/OrganizationSuggestionServiceCore.js b/src/main/services/organization/OrganizationSuggestionServiceCore.js index 6769403..b36f553 100644 --- a/src/main/services/organization/OrganizationSuggestionServiceCore.js +++ b/src/main/services/organization/OrganizationSuggestionServiceCore.js @@ -1884,13 +1884,6 @@ class OrganizationSuggestionServiceCore { const adjusted = rankedSuggestions.map((suggestion) => { const suggestionFolder = suggestion.folder || suggestion.path; - const baseScore = Number.isFinite(suggestion.weightedScore) - ? suggestion.weightedScore - : Number.isFinite(suggestion.score) - ? suggestion.score - : Number.isFinite(suggestion.confidence) - ? suggestion.confidence - : 0; let boost = 0; let penalty = 0; @@ -1924,27 +1917,42 @@ class OrganizationSuggestionServiceCore { } } - const adjustedScore = Math.max(0, Math.min(1, baseScore + boost - penalty)); - const updated = { - ...suggestion, - score: adjustedScore, - confidence: adjustedScore, - weightedScore: adjustedScore - }; + if (boost === 0 && penalty === 0) { + return suggestion; + } - if (boost > 0 || penalty > 0) { - updated.memoryAdjustment = { + // `weightedScore` is the ranking key (score x source weight, up to 1.5) and + // `confidence` is what auto-organize compares with its threshold. Adjust + // each on its own scale: writing the ranking key into confidence moved a + // 0.69 match past 0.75. The ranking key is not capped at 1, or every + // strong suggestion would tie. + const delta = boost - penalty; + const score = Number.isFinite(suggestion.score) + ? suggestion.score + : Number.isFinite(suggestion.confidence) + ? suggestion.confidence + : 0; + const confidence = Number.isFinite(suggestion.confidence) ? suggestion.confidence : score; + const rankingScore = Number.isFinite(suggestion.weightedScore) + ? suggestion.weightedScore + : score; + + return { + ...suggestion, + score: Math.max(0, Math.min(1, score + delta)), + confidence: Math.max(0, Math.min(1, confidence + delta)), + weightedScore: Math.max(0, rankingScore + delta), + memoryAdjustment: { boost, penalty - }; - } - - return updated; + } + }; }); - return adjusted.sort( - (a, b) => (b.weightedScore || b.score || 0) - (a.weightedScore || a.score || 0) - ); + /** @param {any} s */ + const rankingKey = (s) => + Number.isFinite(s.weightedScore) ? s.weightedScore : Number.isFinite(s.score) ? s.score : 0; + return adjusted.sort((a, b) => rankingKey(b) - rankingKey(a)); } async rebuildFeedbackMemoryEmbeddings() { diff --git a/src/preload/ipcSanitizer.js b/src/preload/ipcSanitizer.js index d0129d7..937dcd8 100644 --- a/src/preload/ipcSanitizer.js +++ b/src/preload/ipcSanitizer.js @@ -155,13 +155,22 @@ function createIpcSanitizer({ log }) { return value; }; + // Keys whose values are prose typed by the user (chat question, search + // query, folder description, feedback note, log message), never a path. + // Prose that mentions a Windows path ("What is in D:\Docs?") matches the + // path heuristic and used to be blanked before it left the renderer. Exact + // names only; every other key keeps the heuristic, and a key containing + // "path" or "file" always wins. + const FREE_TEXT_KEYS = new Set(['query', 'description', 'text', 'message', 'note']); + /** * @param {any} obj * @param {boolean} [isFilePath] * @param {number} [_depth] + * @param {boolean} [isFreeText] - value sits under a FREE_TEXT_KEYS key * @returns {any} */ - const sanitizeObject = (obj, isFilePath = false, _depth = 0) => { + const sanitizeObject = (obj, isFilePath = false, _depth = 0, isFreeText = false) => { if (_depth > MAX_SANITIZE_DEPTH) { log.warn('[SecureIPC] Sanitization depth limit reached, returning safe fallback value'); return depthLimitedFallback(obj); @@ -172,7 +181,7 @@ function createIpcSanitizer({ log }) { if (!isFilePath && looksLikeSimpleText(stripped)) { return stripped; } - if (isFilePath || looksLikeFilePath(stripped)) { + if (isFilePath || (!isFreeText && looksLikeFilePath(stripped))) { // Only strip ? and * on Windows where they are invalid in filenames. // process.platform may be undefined in bundled preload (process/browser polyfill), // so fall back to navigator.platform which is always available in Electron's renderer context. @@ -230,7 +239,7 @@ function createIpcSanitizer({ log }) { } if (Array.isArray(obj)) { - return obj.map((item) => sanitizeObject(item, isFilePath, _depth + 1)); + return obj.map((item) => sanitizeObject(item, isFilePath, _depth + 1, isFreeText)); } if (obj && typeof obj === 'object') { @@ -256,7 +265,8 @@ function createIpcSanitizer({ log }) { continue; } - sanitized[cleanKey] = sanitizeObject(value, isPathKey, _depth + 1); + const isFreeTextKey = !isPathKey && FREE_TEXT_KEYS.has(key.toLowerCase()); + sanitized[cleanKey] = sanitizeObject(value, isPathKey, _depth + 1, isFreeTextKey); } return sanitized; } diff --git a/src/renderer/App.js b/src/renderer/App.js index c288d44..c980868 100644 --- a/src/renderer/App.js +++ b/src/renderer/App.js @@ -17,6 +17,7 @@ import { fetchLicenseStatus } from './store/slices/licenseSlice'; import { useSettingsSubscription } from './hooks/useSettingsSubscription'; import { useNotificationActivation } from './hooks/useNotificationActivation'; import { useKeyboardShortcuts } from './hooks/useKeyboardShortcuts'; +import { useNotification } from './contexts/NotificationContext'; import { hasDestinationFolder } from './utils/smartFolderDestinations'; // AppShell is a .jsx module (untyped under checkJs); cast at import so JSX @@ -91,6 +92,24 @@ function AppContent() { useSettingsSubscription(handleSettingsSync, { enabled: Boolean(settingsEventsEnabled) }); useNotificationActivation(); + // Raised here, not in WelcomePhase: store/index.js keeps viewMode across a + // TTL expiry, so a dashboard user never mounts a phase at startup and the + // session reset went unexplained. Single consumer — the flag is cleared so + // the notice cannot repeat. + const { addNotification } = useNotification(); + useEffect(() => { + const win = /** @type {any} */ (window); + if (win.__STRATOSORT_STATE_EXPIRED__) { + const ageHours = win.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__ || 24; + addNotification( + `Previous session (${ageHours}h old) was cleared to ensure fresh data. Your files are safe.`, + 'info' + ); + delete win.__STRATOSORT_STATE_EXPIRED__; + delete win.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__; + } + }, [addNotification]); + // Apply the chosen palette as a data-theme attribute on . The stylesheet // re-themes every token under :root[data-theme='light'|'dark']; "primary" is // the base :root. selectedTheme is persisted by the Redux persistence diff --git a/src/renderer/components/ModelSetupWizard.jsx b/src/renderer/components/ModelSetupWizard.jsx index 546c717..69e6072 100644 --- a/src/renderer/components/ModelSetupWizard.jsx +++ b/src/renderer/components/ModelSetupWizard.jsx @@ -38,12 +38,37 @@ const STEP_TITLES = { downloading: 'Download', complete: 'Finish' }; +const DOWNLOADS_STOPPED_MESSAGE = 'Required model downloads stopped. Please retry downloads.'; +// Main says "started" before the download is in its status list (it checks +// disk space and any partial file first), so for this long after a request an +// empty list does not mean the download stopped. +const DOWNLOAD_START_GRACE_MS = 3000; +const PROJECTOR_MISSING_MESSAGE = + 'The model downloaded, but its image projector file did not download. Images cannot be analyzed until it does. Retry to fetch it.'; const CHECKING_STATUS_MESSAGES = [ 'Detecting available local models...', 'Checking active background downloads...', 'Preparing safe defaults for your setup...' ]; +/** + * The model list answer when the AI engine failed to start: main sends an + * empty list plus the reason. That list says nothing about what is on disk. + * @param {any} modelsResponse + * @returns {string|null} the reason, or null when the list can be trusted + */ +function getModelListError(modelsResponse) { + if (!modelsResponse || Array.isArray(modelsResponse)) return null; + const { error } = modelsResponse; + if (!error) return null; + return toErrorMessage(error, 'Unknown error'); +} + +/** @param {string} detail */ +function describeEngineFailure(detail) { + return `The AI engine could not start, so installed models cannot be checked. Press Refresh to try again, or continue with limited AI. Details: ${detail}`; +} + function getModelTotalBytes(filename) { const model = getModel(filename); if (!model) return null; @@ -208,6 +233,15 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { // first one was still enumerating models. This ref flips immediately on // entry and resets in `finally`. const startDownloadsInFlightRef = useRef(false); + // When main last acknowledged a download request (see DOWNLOAD_START_GRACE_MS). + const lastDownloadAckAtRef = useRef(0); + // Vision models whose GGUF arrived without its projector (mmproj) file: + // model filename -> projector filename (null when unknown). Main lists the + // GGUF as installed, so the list alone must not mark these ready. + const missingProjectorRef = useRef(/** @type {Map} */ (new Map())); + // True when the engine failed to start and the model list is therefore + // empty for a reason other than "nothing is installed". + const [modelListUnknown, setModelListUnknown] = useState(false); // Embedding model the engine is configured with when the wizard loaded, // and the pending "switch and rebuild?" question, if any (audit row ML-08). const currentEmbeddingModelRef = useRef(null); @@ -250,11 +284,32 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { })); }, []); + // Installed names from main, minus vision models still waiting for their + // projector. A list that names the projector settles the matter. + const withoutMissingProjector = useCallback((names) => { + const missing = missingProjectorRef.current; + if (missing.size === 0) return names; + const listed = new Set(names); + for (const [model, projector] of missing) { + if (projector && listed.has(projector)) missing.delete(model); + } + return names.filter((name) => !missing.has(name)); + }, []); + + const downloadWasJustRequested = useCallback( + () => + startDownloadsInFlightRef.current || + Date.now() - lastDownloadAckAtRef.current < DOWNLOAD_START_GRACE_MS, + [] + ); + const handleCancelDownload = useCallback( async (filename) => { if (!filename || !window?.electronAPI?.llama?.cancelDownload) return; try { - const result = await window.electronAPI.llama.cancelDownload(filename); + // A projector-only retry downloads the projector file, not the model. + const target = missingProjectorRef.current.get(filename) || filename; + const result = await window.electronAPI.llama.cancelDownload(target); if (result?.success !== false) { updateDownloadState(filename, { status: 'cancelled', error: null }); } @@ -438,7 +493,17 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { 'AI system check' ); if (!isMountedRef.current) return; - const config = configResponse?.config || configResponse || {}; + // A failed engine answers getConfig with an error envelope, not a config. + const configFailed = configResponse?.success === false; + const config = configFailed ? {} : configResponse?.config || configResponse || {}; + const modelListError = getModelListError(modelsResponse); + const engineError = + modelListError || + (configFailed ? toErrorMessage(configResponse?.error, 'Unknown error') : null); + setModelListUnknown(Boolean(modelListError)); + if (engineError) { + setInitError(describeEngineFailure(engineError)); + } const suggestedSelection = modelsResponse?.selected || {}; currentEmbeddingModelRef.current = @@ -459,7 +524,9 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { : Array.isArray(modelsResponse?.models) ? modelsResponse.models : []; - const available = modelList.map((m) => m.name || m.filename || m).filter(Boolean); + const available = withoutMissingProjector( + modelList.map((m) => m.name || m.filename || m).filter(Boolean) + ); const availableNow = new Set(available); setSelectedProfile(profileKey); @@ -533,7 +600,10 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { .filter(Boolean) .filter((name) => !availableNow.has(name)); - if (missingSelected.length === 0) { + if (modelListError) { + // Nothing is known about what is installed; the reason is on screen. + setStep('select'); + } else if (missingSelected.length === 0) { // All profile models already on disk: still apply the profile so // LlamaService config matches the selection. Skipping this left the // wizard "complete" while the engine kept stale model settings. @@ -553,6 +623,7 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { } catch (error) { if (!isMountedRef.current) return; applyFallbackSelection(); + setModelListUnknown(false); if ( typeof error?.message === 'string' && error.message.startsWith('AI system check timed out') @@ -569,7 +640,7 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { setIsRefreshing(false); } } - }, [applyProfileConfig]); + }, [applyProfileConfig, withoutMissingProjector]); useEffect(() => { void checkSystem(); @@ -627,6 +698,53 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { const modelName = data.model || payload.model || payload.filename; if (!modelName) return; + // A projector fetched on its own (the retry for a vision model that + // arrived without one) reports under its own filename; the row on + // screen belongs to the vision model. + const missingProjector = missingProjectorRef.current; + const projectorOwner = [...missingProjector.keys()].find( + (owner) => missingProjector.get(owner) === modelName + ); + if (projectorOwner) { + if (eventType === 'model-download') { + updateDownloadState(projectorOwner, { + status: 'downloading', + percent: payload.percent ?? payload.percentage ?? 0, + downloadedBytes: payload.downloadedBytes, + totalBytes: payload.totalBytes, + speedBps: payload.speedBps, + etaSeconds: payload.etaSeconds, + error: null + }); + } else if (eventType === 'model-download-complete') { + missingProjector.delete(projectorOwner); + const ownerBytes = getModelTotalBytes(projectorOwner); + updateDownloadState(projectorOwner, { + status: 'ready', + percent: 100, + downloadedBytes: ownerBytes, + totalBytes: ownerBytes, + speedBps: 0, + etaSeconds: 0, + error: null + }); + setAvailableModels((prev) => Array.from(new Set([...(prev || []), projectorOwner]))); + } else if (eventType === 'model-download-error') { + setDownloadState((prev) => { + if (prev[projectorOwner]?.status === 'cancelled') return prev; + return { + ...prev, + [projectorOwner]: { + ...(prev[projectorOwner] || {}), + status: 'failed', + error: PROJECTOR_MISSING_MESSAGE + } + }; + }); + } + return; + } + if (eventType === 'model-download') { updateDownloadState(modelName, { status: 'downloading', @@ -640,6 +758,24 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { } if (eventType === 'model-download-complete') { + // Main reports a vision model whose projector failed to download as + // complete but degraded (success: false). The GGUF alone cannot + // analyze images, so the model is not ready. + if (data.degraded === true || payload.degraded === true) { + missingProjector.set( + modelName, + payload.companion?.name || getModel(modelName)?.clipModel?.name || null + ); + updateDownloadState(modelName, { + status: 'failed', + speedBps: 0, + etaSeconds: 0, + error: PROJECTOR_MISSING_MESSAGE + }); + setAvailableModels((prev) => (prev || []).filter((name) => name !== modelName)); + return; + } + missingProjector.delete(modelName); const totalBytes = typeof payload.totalBytes === 'number' && payload.totalBytes > 0 ? payload.totalBytes @@ -713,8 +849,11 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { : Array.isArray(modelsResponse?.models) ? modelsResponse.models : []; - const latestAvailable = modelList.map((m) => m.name || m.filename || m).filter(Boolean); + const latestAvailable = withoutMissingProjector( + modelList.map((m) => m.name || m.filename || m).filter(Boolean) + ); const latestAvailableSet = new Set(latestAvailable); + const modelListError = getModelListError(modelsResponse); const hasReliableDownloadStatus = Array.isArray(downloadStatus?.status?.downloads); const activeDownloads = hasReliableDownloadStatus ? downloadStatus.status.downloads : []; @@ -750,6 +889,17 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { return next; }); + if (modelListError) { + // The engine failed to start, so the empty list is not "nothing + // installed" and finished downloads are not "stopped". Say why + // setup cannot finish and leave the rows to the download events. + const engineMessage = describeEngineFailure(modelListError); + setInitError((prev) => + !prev || prev === DOWNLOADS_STOPPED_MESSAGE ? engineMessage : prev + ); + return; + } + const selectedMissing = Object.values(selectedModels) .filter(Boolean) .filter((name) => !latestAvailableSet.has(name)); @@ -770,11 +920,11 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { if ( requiredMissing.length > 0 && hasReliableDownloadStatus && - activeDownloads.length === 0 + activeDownloads.length === 0 && + // A request main has only just acknowledged is not in its list yet. + !downloadWasJustRequested() ) { - setInitError( - (prev) => prev || 'Required model downloads stopped. Please retry downloads.' - ); + setInitError((prev) => prev || DOWNLOADS_STOPPED_MESSAGE); setStep('select'); } } catch { @@ -791,7 +941,13 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { cancelled = true; clearInterval(timer); }; - }, [step, selectedModels, applySelectedProfileConfig]); + }, [ + step, + selectedModels, + applySelectedProfileConfig, + withoutMissingProjector, + downloadWasJustRequested + ]); async function startDownloads() { // Synchronous double-trigger guard. Even with `disabled` on the button @@ -811,9 +967,10 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { } async function runStartDownloads() { + const missingProjector = missingProjectorRef.current; const modelsToDownload = Object.values(selectedModels) .filter(Boolean) - .filter((modelName) => !availableSet.has(modelName)); + .filter((modelName) => !availableSet.has(modelName) || missingProjector.has(modelName)); if (modelsToDownload.length === 0) { const applied = await applySelectedProfileConfig(); @@ -829,14 +986,22 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { } userInitiatedDownloadsRef.current = true; + // A new attempt replaces the verdict on the last one. + setInitError((prev) => (prev === DOWNLOADS_STOPPED_MESSAGE ? null : prev)); setStep('downloading'); - const nextAvailable = new Set(availableSet); + const nextAvailable = new Set([...availableSet].filter((name) => !missingProjector.has(name))); + let acknowledged = 0; for (const filename of modelsToDownload) { updateDownloadState(filename, { status: 'downloading', percent: 0, error: null }); try { - const result = await window.electronAPI.llama.downloadModel(filename); + // The GGUF of a vision model that only lacks its projector is already + // on disk: fetch the projector alone instead of the whole model again. + const target = missingProjector.get(filename) || filename; + const result = await window.electronAPI.llama.downloadModel(target); if (result?.success) { + acknowledged += 1; + lastDownloadAckAtRef.current = Date.now(); // downloadModel() acknowledges that a download has started; it does not // guarantee the model is installed yet. We only mark ready when the // model appears in getModels() or a completion event arrives. @@ -859,6 +1024,7 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { // This avoids treating "alreadyInProgress" downloads as fully installed. let activeDownloads = []; let hasReliableDownloadStatus = false; + let modelListError = null; try { const [modelsResponse, downloadStatus] = await Promise.all([ window.electronAPI.llama.getModels(), @@ -875,7 +1041,10 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { : Array.isArray(modelsResponse?.models) ? modelsResponse.models : []; - const latestAvailable = modelList.map((m) => m.name || m.filename || m).filter(Boolean); + const latestAvailable = withoutMissingProjector( + modelList.map((m) => m.name || m.filename || m).filter(Boolean) + ); + modelListError = getModelListError(modelsResponse); latestAvailable.forEach((name) => nextAvailable.add(name)); if (latestAvailable.length > 0) { @@ -912,13 +1081,15 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { if (selectedMissing.length === 0 && configApplied) { setStep('complete'); } else { - setStep( - hasReliableDownloadStatus - ? activeDownloads.length > 0 - ? 'downloading' - : 'select' - : 'downloading' - ); + // An empty status list right after main acknowledged a request only + // means the download is not registered yet; with a failed engine the + // model list cannot say what is missing either. The poll takes over. + const statusSaysNothingRuns = + hasReliableDownloadStatus && + activeDownloads.length === 0 && + acknowledged === 0 && + !modelListError; + setStep(statusSaysNothingRuns ? 'select' : 'downloading'); } } @@ -1134,7 +1305,11 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { selected={selectedModels.embedding} recommendations={recommendations} onChange={(f) => toggleModel('embedding', f)} - status={downloadState[selectedModels.embedding]?.status} + status={ + modelListUnknown && !downloadState[selectedModels.embedding]?.status + ? 'unknown' + : downloadState[selectedModels.embedding]?.status + } error={downloadState[selectedModels.embedding]?.error} disabled={!hasApi || isRefreshing} required @@ -1148,7 +1323,11 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { selected={selectedModels.text} recommendations={recommendations} onChange={(f) => toggleModel('text', f)} - status={downloadState[selectedModels.text]?.status} + status={ + modelListUnknown && !downloadState[selectedModels.text]?.status + ? 'unknown' + : downloadState[selectedModels.text]?.status + } error={downloadState[selectedModels.text]?.error} disabled={!hasApi || isRefreshing} required @@ -1162,7 +1341,11 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { selected={selectedModels.vision} recommendations={recommendations} onChange={(f) => toggleModel('vision', f)} - status={downloadState[selectedModels.vision]?.status} + status={ + modelListUnknown && !downloadState[selectedModels.vision]?.status + ? 'unknown' + : downloadState[selectedModels.vision]?.status + } error={downloadState[selectedModels.vision]?.error} disabled={!hasApi || isRefreshing} optional @@ -1191,7 +1374,7 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { - {requiredModelsMissing.length > 0 && ( + {requiredModelsMissing.length > 0 && !modelListUnknown && ( Required models are missing. Download them to enable AI features. @@ -1202,7 +1385,15 @@ function ModelSetupWizardBody({ onComplete, onSkip, onDismiss, onStepChange }) { onClick={startDownloads} variant="primary" className="flex-1" - disabled={!selectedModels.embedding || !selectedModels.text || !hasApi || isRefreshing} + disabled={ + !selectedModels.embedding || + !selectedModels.text || + !hasApi || + isRefreshing || + // Downloading again what may already be on disk helps nobody + // while the engine that would load it cannot start. + modelListUnknown + } > Download models @@ -1542,7 +1733,13 @@ function ModelSelector({ const displayName = modelInfo?.displayName || filename; const modelSize = getModelSize(filename); const isInstalled = status === 'ready' || status === 'complete'; - const statusLabel = isInstalled ? 'Installed' : status === 'failed' ? 'Failed' : 'Not installed'; + const statusLabel = isInstalled + ? 'Installed' + : status === 'failed' + ? 'Failed' + : status === 'unknown' + ? 'Not checked' + : 'Not installed'; const statusClass = isInstalled ? 'bg-stratosort-success/10 text-stratosort-success' : status === 'failed' diff --git a/src/renderer/components/SettingsPanel.jsx b/src/renderer/components/SettingsPanel.jsx index b89176e..70eefbc 100644 --- a/src/renderer/components/SettingsPanel.jsx +++ b/src/renderer/components/SettingsPanel.jsx @@ -137,6 +137,17 @@ const EXPERIENCE_MODE_COPY = { 'Full control over local models, embeddings, retrieval, and diagnostics.' }; +/** + * Text for a vision model that installed but whose projector (mmproj) failed + * to download: the model is listed, yet it cannot analyze images. + */ +function projectorMissingMessage(modelName, evt) { + const projector = evt?.progress?.companion?.name; + return `Model "${modelName}" installed, but its image projector${ + projector ? ` (${projector})` : '' + } did not download. Image analysis will not work until it does.`; +} + /** * Format a "saved " string without registering a tick interval. * The chip re-renders whenever lastSavedAt updates (next save, next @@ -384,6 +395,10 @@ const SettingsPanel = React.memo(function SettingsPanel() { const [lastSaveError, setLastSaveError] = useState(null); const [searchQuery, setSearchQuery] = useState(''); const [settingsLoaded, setSettingsLoaded] = useState(false); + // Set while the read from main has failed: the form below still holds + // defaults, so nothing may be saved until a read succeeds. + const [settingsLoadError, setSettingsLoadError] = useState(null); + const [isRetryingSettingsLoad, setIsRetryingSettingsLoad] = useState(false); const [isHydrating, setIsHydrating] = useState(true); const [newModel, setNewModel] = useState(''); const [isAddingModel, setIsAddingModel] = useState(false); @@ -417,6 +432,7 @@ const SettingsPanel = React.memo(function SettingsPanel() { const cancelAutoSaveRef = useRef(null); const settingsRef = useRef(null); const settingsLoadedRef = useRef(false); + const preloadFormRef = useRef(null); const modelLoadRequestRef = useRef(0); const lastSavedSnapshotRef = useRef(computeSettingsSnapshot(DEFAULT_SETTINGS)); const pendingExternalSnapshotRef = useRef(null); @@ -503,10 +519,14 @@ const SettingsPanel = React.memo(function SettingsPanel() { // can start editing the DEFAULT_SETTINGS form. Capture the pre-fetch form // state so the late-arriving persisted settings don't clobber those // in-flight edits — any key that changed while we awaited is overlaid - // back on top of the fetched values (and then auto-saved). - const preloadForm = { ...(settingsRef.current || DEFAULT_SETTINGS) }; + // back on top of the fetched values (and then auto-saved). The baseline + // is kept across failed attempts, so an edit made before a retry counts. + if (settingsLoadedRef.current) return; + const preloadForm = preloadFormRef.current || { + ...(settingsRef.current || DEFAULT_SETTINGS) + }; + preloadFormRef.current = preloadForm; try { - if (settingsLoadedRef.current) return; // Always fetch authoritative settings from main-process persistence. // Relying on cached Redux ui.settings can replay stale/default values and // accidentally overwrite user-selected locations during auto-save. @@ -555,6 +575,8 @@ const SettingsPanel = React.memo(function SettingsPanel() { } dispatch(updateSettings(settingsWithoutLiveActivity(mergedSettings))); updateLastSavedSnapshot(mergedSettings); + preloadFormRef.current = null; + setSettingsLoadError(null); setSettingsLoaded(true); settingsLoadedRef.current = true; } catch (error) { @@ -562,14 +584,29 @@ const SettingsPanel = React.memo(function SettingsPanel() { error: error.message, stack: error.stack }); - skipAutoSaveRef.current += 1; - applySettingsUpdate({ ...DEFAULT_SETTINGS }); - dispatch(updateSettings(settingsWithoutLiveActivity({ ...DEFAULT_SETTINGS }))); - updateLastSavedSnapshot(DEFAULT_SETTINGS); - setSettingsLoaded(true); - settingsLoadedRef.current = true; + // Fail closed. The form still holds DEFAULT_SETTINGS; marking it loaded + // would let the next edit auto-save the whole default form over the + // stored settings, and dispatching it would make the rest of the app + // read defaults as if they were the user's. Stay unloaded (auto-save is + // gated on that) and say so, with a way to try again. + setSettingsLoadError(toErrorMessage(error, 'unknown error')); + addNotification( + "Couldn't load your settings. Nothing will be saved until they load.", + 'error', + 6000, + 'settings-load-error' + ); + } + }, [addNotification, applySettingsUpdate, dispatch, updateLastSavedSnapshot]); + + const retryLoadSettings = useCallback(async () => { + setIsRetryingSettingsLoad(true); + try { + await loadSettings(); + } finally { + setIsRetryingSettingsLoad(false); } - }, [applySettingsUpdate, dispatch, updateLastSavedSnapshot]); + }, [loadSettings]); const loadModels = useCallback(async () => { const requestId = modelLoadRequestRef.current + 1; @@ -829,6 +866,8 @@ const SettingsPanel = React.memo(function SettingsPanel() { const autoSaveSettings = useDebouncedCallback(async () => { const currentSettings = settingsRef.current; if (!currentSettings) return; + // Never save a form that was not loaded from disk (see loadSettings). + if (!settingsLoadedRef.current) return; try { setIsSaving(true); @@ -1058,7 +1097,11 @@ const SettingsPanel = React.memo(function SettingsPanel() { }); } else if (evt.type === 'model-download-complete') { setPullProgress({ modelName: requested, filename: evt.model, percent: 100 }); - addNotification(`Model "${requested}" installed`, 'success'); + if (evt.degraded) { + addNotification(projectorMissingMessage(requested, evt), 'warning'); + } else { + addNotification(`Model "${requested}" installed`, 'success'); + } setNewModel(''); // Refresh on completion — not on IPC ack — so the list reflects // the file actually being on disk. @@ -1185,7 +1228,11 @@ const SettingsPanel = React.memo(function SettingsPanel() { }); } else if (evt?.type === 'model-download-complete' && pendingModels.has(evt.model)) { pendingModels.delete(evt.model); - addNotification(`Model "${evt.model}" downloaded successfully`, 'success'); + if (evt.degraded) { + addNotification(projectorMissingMessage(evt.model, evt), 'warning'); + } else { + addNotification(`Model "${evt.model}" downloaded successfully`, 'success'); + } loadModels(); if (pendingModels.size === 0) { finishAll(); @@ -1524,6 +1571,25 @@ const SettingsPanel = React.memo(function SettingsPanel() { )} + {settingsLoadError && !settingsLoaded && ( +
+ + + +
+ )} {pendingExternalSettings && ( 0; const percentToShow = completedFlash @@ -252,9 +258,7 @@ function OrganizeProgress({ {completedFlash ? 'Done!' : 'Organizing files...'} - {completedFlash - ? `Organized ${displayCurrent} ${displayCurrent === 1 ? 'file' : 'files'}.` - : 'Do not close the app until completion'} + {completedFlash ? completedText : 'Do not close the app until completion'} @@ -358,8 +362,14 @@ function OrganizeProgress({
{preview.slice(0, 6).map((op, idx) => { + // Progress names the file by its original name; fileName here is + // the name it will have after the move. + const sourceName = + typeof op.sourcePath === 'string' && op.sourcePath + ? op.sourcePath.split(/[\\/]/).pop() + : op.fileName; const isCurrentFile = - batchProgress.currentFile && op.fileName === batchProgress.currentFile; + Boolean(batchProgress.currentFile) && sourceName === batchProgress.currentFile; const isProcessed = hasTotals && idx < batchProgress.current; return ( @@ -420,6 +430,7 @@ OrganizeProgress.propTypes = { preview: PropTypes.arrayOf( PropTypes.shape({ fileName: PropTypes.string, + sourcePath: PropTypes.string, destination: PropTypes.string }) ), @@ -433,7 +444,8 @@ OrganizeProgress.propTypes = { remaining: PropTypes.number }), onResume: PropTypes.func, - onDismissPaused: PropTypes.func + onDismissPaused: PropTypes.func, + organizedCount: PropTypes.number }; export default OrganizeProgress; diff --git a/src/renderer/components/search/ChatPanel.jsx b/src/renderer/components/search/ChatPanel.jsx index 7047af8..74ae311 100644 --- a/src/renderer/components/search/ChatPanel.jsx +++ b/src/renderer/components/search/ChatPanel.jsx @@ -274,18 +274,23 @@ function stripUnresolvedCitations(text, sources) { } function getAssistantDisplayText(message) { - if (typeof message?.text === 'string' && message.text.trim()) { - // Streaming path: the finished message swaps `.text` for the structured, - // source-reconciled answer, so this branch only renders mid-stream text. - return stripUnresolvedCitations( - sanitizeAnswerText(message.text, message.sources), - message.sources - ); - } - - return getPrimaryAnswerItems(message) - .map((item) => item.text) - .join('\n\n'); + const hasText = typeof message?.text === 'string' && message.text.trim(); + // A finished message renders its structured, source-reconciled answer. A + // message reopened from history still carries `.text` (the prompt-memory + // string, with model text and a "[Referenced: ...]" line), so `.text` is + // shown only mid-stream or when there is no structured answer. + const structuredText = + hasText && message.isStreaming + ? '' + : getPrimaryAnswerItems(message) + .map((item) => item.text) + .join('\n\n'); + if (structuredText || !hasText) return structuredText; + + return stripUnresolvedCitations( + sanitizeAnswerText(message.text, message.sources), + message.sources + ); } const LONG_THREAD_MESSAGE_THRESHOLD = 45; diff --git a/src/renderer/components/search/DocumentScopePanel.jsx b/src/renderer/components/search/DocumentScopePanel.jsx index 3677ee8..5e490b6 100644 --- a/src/renderer/components/search/DocumentScopePanel.jsx +++ b/src/renderer/components/search/DocumentScopePanel.jsx @@ -4,6 +4,7 @@ import { X, FileText, Folder, Plus, FolderInput, ChevronRight } from 'lucide-rea import { Button, IconButton } from '../ui'; import { Text } from '../ui/Typography'; import { formatDisplayPath } from '../../utils/pathDisplay'; +import { resolveDroppedFilePath } from '../../utils/dragAndDrop'; import { useSelector } from 'react-redux'; import { selectRedactPaths } from '../../store/selectors'; @@ -81,10 +82,22 @@ export default function DocumentScopePanel({ const droppedFiles = e.dataTransfer?.files; if (droppedFiles && droppedFiles.length > 0) { - const files = Array.from(droppedFiles).map((f) => ({ - path: f.path, + // File-kind items line up with dataTransfer.files; their entries are + // the only synchronous way to tell a dropped folder from a file. + const entries = Array.from(e.dataTransfer?.items || []) + .filter((item) => item?.kind === 'file') + .map((item) => { + try { + return item.webkitGetAsEntry?.() || null; + } catch { + return null; + } + }); + const files = Array.from(droppedFiles).map((f, index) => ({ + // File.path is gone in modern Electron; the preload bridge resolves it. + path: resolveDroppedFilePath(f), name: f.name, - type: 'file' + type: entries[index]?.isDirectory ? 'folder' : 'file' })); onAddToScope(files); } diff --git a/src/renderer/components/search/TimelineTabContent.jsx b/src/renderer/components/search/TimelineTabContent.jsx index e78f005..f204e96 100644 --- a/src/renderer/components/search/TimelineTabContent.jsx +++ b/src/renderer/components/search/TimelineTabContent.jsx @@ -5,7 +5,7 @@ * onOpenFile. Renders a virtualized list of year/month/undated headers and * item rows built by buildTimelineRows(). */ -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import { List as VirtualizedList } from 'react-window'; import { buildTimelineRows, formatTimelineDay } from '../../utils/timelineRows'; @@ -125,11 +125,14 @@ export default function TimelineTabContent({ loadMore, onOpenFile }) { - const containerRef = useRef(null); + // The measured container only exists while there are rows, so it is held in + // state: the observer attaches when the first page arrives and re-attaches + // after a refresh emptied the list. + const [containerEl, setContainerEl] = useState(/** @type {HTMLDivElement | null} */ (null)); const [height, setHeight] = useState(400); useEffect(() => { - const el = containerRef.current; + const el = containerEl; if (!el) return; const ro = new ResizeObserver((entries) => { const entry = entries[0]; @@ -140,7 +143,7 @@ export default function TimelineTabContent({ }); ro.observe(el); return () => ro.disconnect(); - }, []); + }, [containerEl]); const rows = useMemo(() => buildTimelineRows(items), [items]); @@ -173,7 +176,7 @@ export default function TimelineTabContent({ )} {rows.length > 0 && ( -
+
)} diff --git a/src/renderer/phases/WelcomePhase.jsx b/src/renderer/phases/WelcomePhase.jsx index 8cb7724..b97a689 100644 --- a/src/renderer/phases/WelcomePhase.jsx +++ b/src/renderer/phases/WelcomePhase.jsx @@ -328,19 +328,6 @@ function WelcomePhase() { } }, [modelCheckState, showLoadingEscape, addNotification]); - useEffect(() => { - if (window.__STRATOSORT_STATE_EXPIRED__) { - const ageHours = window.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__ || 24; - addNotification( - `Previous session (${ageHours}h old) was cleared to ensure fresh data. Your files are safe.`, - 'info' - ); - // Clear flag so notification doesn't repeat - delete window.__STRATOSORT_STATE_EXPIRED__; - delete window.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__; - } - }, [addNotification]); - const actions = { advancePhase: (phase) => dispatch(setPhase(phase)) }; diff --git a/src/renderer/phases/discover/useAnalysis.js b/src/renderer/phases/discover/useAnalysis.js index 5c4354b..f85e725 100644 --- a/src/renderer/phases/discover/useAnalysis.js +++ b/src/renderer/phases/discover/useAnalysis.js @@ -37,7 +37,12 @@ import { import { mapErrorToNotification } from '../../utils/errorMapping'; import { normalizeNamingConventionState } from '../../../shared/namingConventions'; import { toCanonicalPath } from '../../utils/canonicalPath'; -import { filesNotYetReady, isBatchAlreadyRunning, runBatchPathWhenFree } from './batchBusy'; +import { + BATCH_BUSY_RETRY_MS, + filesNotYetReady, + isBatchAlreadyRunning, + runBatchPathWhenFree +} from './batchBusy'; import { setAnalysisRunBase, clearAnalysisRunBase } from '../../store/analysisRunBase'; const logger = createLogger('DiscoverPhase:Analysis'); @@ -45,6 +50,10 @@ const BATCH_ANALYSIS_MIN_FILES = 40; /** Mixed-modality batch threshold when hardware recommends sequential (low VRAM). */ const BATCH_ANALYSIS_LOW_VRAM_MIN_FILES = 3; const BATCH_ANALYSIS_PROGRESS_EVENT = 'operation-progress'; +// How long a recording waits for the media request of an earlier one to end. +// The preload gives that request 330 s and the per-file path 295 s, so a +// request can outlive its file by 35 s. +const MEDIA_BUSY_MAX_WAIT_MS = 42 * 1000; const MEDIA_EXTENSIONS = new Set([ ...(SUPPORTED_AUDIO_EXTENSIONS || []), ...(SUPPORTED_VIDEO_EXTENSIONS || []) @@ -69,13 +78,17 @@ function clearAutoAdvanceTimeoutRef(autoAdvanceTimeoutRef) { * * @param {string} filePath - Path to the file to analyze * @param {number} attempt - Current attempt number (1-based) + * @param {any} [abortSignal] + * @param {any} [namingSettings] + * @param {number} [mediaBusyWaitedMs] - time already spent waiting for an earlier media request * @returns {Promise} Analysis result */ async function analyzeWithRetry( filePath, attempt = 1, - /** @type {any} */ abortSignal = null, - /** @type {any} */ namingSettings = null + abortSignal = null, + namingSettings = null, + mediaBusyWaitedMs = 0 ) { // Ensure electronAPI is available before attempting analysis if (!window.electronAPI?.files?.analyze) { @@ -96,6 +109,32 @@ async function analyzeWithRetry( // service-restart that surfaces as an abort is retried instead of being // bubbled up as a hard failure. const message = String(error?.message || ''); + + // The preload runs one media request per window. A recording that hit + // its limit here still holds that request until the preload's own, + // slightly longer timeout, so the next recording is refused for a short + // while. Wait for it; this is not a failed analysis and uses no attempt. + if ( + isMediaRequestBusy(error) && + isMediaFilePath(filePath) && + mediaBusyWaitedMs < MEDIA_BUSY_MAX_WAIT_MS + ) { + if (abortSignal?.aborted) { + throw new Error('Analysis cancelled by user', { cause: error }); + } + await new Promise((r) => setTimeout(r, BATCH_BUSY_RETRY_MS)); + if (abortSignal?.aborted) { + throw new Error('Analysis cancelled by user', { cause: error }); + } + return analyzeWithRetry( + filePath, + attempt, + abortSignal, + namingSettings, + mediaBusyWaitedMs + BATCH_BUSY_RETRY_MS + ); + } + const isTransient = message.includes('timeout') || message.includes('aborted'); if (attempt < RETRY.MAX_ATTEMPTS_MEDIUM && isTransient) { @@ -109,12 +148,30 @@ async function analyzeWithRetry( if (abortSignal?.aborted) { throw new Error('Analysis cancelled by user', { cause: error }); } - return analyzeWithRetry(filePath, attempt + 1, abortSignal, namingSettings); + return analyzeWithRetry( + filePath, + attempt + 1, + abortSignal, + namingSettings, + mediaBusyWaitedMs + ); } throw error; } } +/** + * The preload's refusal reaches this code through contextBridge, which copies + * an Error's message but not its `code`. The message is what identifies it. + * @param {any} error + * @returns {boolean} the media request was refused because one is still pending + */ +function isMediaRequestBusy(error) { + if (!error) return false; + if (error.code === 'CONCURRENCY_LIMIT') return true; + return /Operation already in progress: analysis:analyze-media/.test(String(error.message || '')); +} + /** * @param {any} filePath * @returns {boolean} an audio or video file, by extension @@ -1584,12 +1641,47 @@ export function useAnalysis(options = {}) { } }; + // The preload runs one analysis:analyze-media request per window and + // refuses a second ("Operation already in progress"), so that speech + // recognition is never loaded twice. Workers here run in parallel: + // recordings take turns, documents and images do not wait. + /** @type {Promise} */ + let mediaTurn = Promise.resolve(); + /** @returns {Promise<() => void>} resolves with the call that ends the turn */ + const takeMediaTurn = () => { + const earlier = mediaTurn; + /** @type {() => void} */ + let endTurn = () => {}; + mediaTurn = new Promise((resolve) => { + endTurn = () => resolve(); + }); + return earlier.then(() => endTurn); + }; + const processFile = async (/** @type {any} */ file) => { if (!isActiveRun() || abortSignal.aborted) return; if (processedFiles.has(file.path)) return; // Result already applied by a streamed batch event — nothing left // to do for this file (see appliedResultPaths above). if (appliedResultPaths.has(toCanonicalPath(file.path))) return; + if (!isMediaFilePath(file.path)) { + await analyzeOneFile(file); + return; + } + // Taken before the file is marked as analyzing and before its time + // limit starts: waiting for a turn is not time spent on the file. + const endTurn = await takeMediaTurn(); + try { + await analyzeOneFile(file); + } finally { + endTurn(); + } + }; + + const analyzeOneFile = async (/** @type {any} */ file) => { + if (!isActiveRun() || abortSignal.aborted) return; + if (processedFiles.has(file.path)) return; + if (appliedResultPaths.has(toCanonicalPath(file.path))) return; const fileName = file.name || file.path.split(/[\\/]/).pop(); processedFiles.add(file.path); @@ -1623,12 +1715,26 @@ export function useAnalysis(options = {}) { // file fails first and the run goes on to the next. lastProgressAtRef.current = Date.now(); + // The limit below ends the wait, not the request. Once it has + // passed, the request must not be sent again when it times out in + // the preload: for a recording that would hold the one media + // request for minutes more and refuse every recording after it. + let gaveUp = false; + const fileSignal = { + get aborted() { + return gaveUp || abortSignal.aborted; + } + }; + try { const analysis = await withTimeout( - analyzeWithRetry(file.path, 1, abortSignal, namingSettings), + analyzeWithRetry(file.path, 1, fileSignal, namingSettings), PER_FILE_LIMIT_MS, `Analysis for ${fileName}` - ); + ).catch((/** @type {any} */ error) => { + gaveUp = true; + throw error; + }); // This prevents state updates if the user cancelled while analysis was in flight if (!isActiveRun() || abortSignal.aborted) return; diff --git a/src/renderer/phases/discover/useFileHandlers.js b/src/renderer/phases/discover/useFileHandlers.js index 4953af1..1f8d564 100644 --- a/src/renderer/phases/discover/useFileHandlers.js +++ b/src/renderer/phases/discover/useFileHandlers.js @@ -794,11 +794,43 @@ export function useFileHandlers({ } const expandedFromFolders = await expandDroppedDirectories(droppedDirectories); - const enhancedFiles = [...supportedDroppedFiles, ...expandedFromFolders]; + + // The check above only saw the dropped folder itself. Its files can + // already be queued (the same folder dropped twice), or be dropped + // alongside it, and must not be reset to pending and analyzed again. + const queuedPaths = new Set( + selectedFiles.map((/** @type {any} */ f) => toCanonicalPath(f.path)) + ); + const droppedPaths = new Set(supportedDroppedFiles.map((f) => toCanonicalPath(f.path))); + let alreadyQueuedCount = 0; + const newFromFolders = expandedFromFolders.filter((/** @type {any} */ file) => { + const canonical = toCanonicalPath(file.path); + if (queuedPaths.has(canonical)) { + alreadyQueuedCount += 1; + return false; + } + if (droppedPaths.has(canonical)) return false; + droppedPaths.add(canonical); + return true; + }); + + const enhancedFiles = [...supportedDroppedFiles, ...newFromFolders]; if (enhancedFiles.length === 0) { - addNotification('No supported files found in drop', 'warning', 2000, 'drop-empty'); + if (alreadyQueuedCount > 0) { + addNotification('All files are already in the queue', 'info', 2000, 'duplicate-files'); + } else { + addNotification('No supported files found in drop', 'warning', 2000, 'drop-empty'); + } return; } + if (alreadyQueuedCount > 0) { + addNotification( + `Skipped ${alreadyQueuedCount} duplicate file${alreadyQueuedCount !== 1 ? 's' : ''} already in queue`, + 'info', + 2000, + 'duplicate-files' + ); + } // Update file states enhancedFiles.forEach((file) => updateFileState(file.path, 'pending')); diff --git a/src/renderer/phases/organize/useFileEditing.js b/src/renderer/phases/organize/useFileEditing.js index 86c4021..6e4641f 100644 --- a/src/renderer/phases/organize/useFileEditing.js +++ b/src/renderer/phases/organize/useFileEditing.js @@ -289,7 +289,7 @@ export function useFileSelection(files) { /** * Hook for bulk operations - * @param {{ selectedFiles: any[], editingFiles: Record, setEditingFiles: Function, setSelectedFiles: Function, addNotification: Function }} params + * @param {{ selectedFiles: any[], editingFiles: Record, setEditingFiles: Function, setSelectedFiles: Function, addNotification: Function, onEditChange?: Function, onBulkEditChange?: Function }} params * @returns {Record} Bulk operation state and handlers */ export function useBulkOperations({ @@ -297,7 +297,9 @@ export function useBulkOperations({ editingFiles, setEditingFiles, setSelectedFiles, - addNotification + addNotification, + onEditChange, + onBulkEditChange }) { const [bulkEditMode, setBulkEditMode] = useState(false); const [bulkCategory, setBulkCategory] = useState(''); @@ -306,6 +308,8 @@ export function useBulkOperations({ const editingFilesRef = useRef(editingFiles); const bulkCategoryRef = useRef(bulkCategory); const addNotificationRef = useRef(addNotification); + const onEditChangeRef = useRef(onEditChange); + const onBulkEditChangeRef = useRef(onBulkEditChange); // Keep refs in sync with latest values useEffect(() => { @@ -320,6 +324,10 @@ export function useBulkOperations({ useEffect(() => { addNotificationRef.current = addNotification; }, [addNotification]); + useEffect(() => { + onEditChangeRef.current = onEditChange; + onBulkEditChangeRef.current = onBulkEditChange; + }, [onEditChange, onBulkEditChange]); // Debounced bulk category change ref const debouncedBulkCategoryChangeRef = useRef(null); @@ -340,10 +348,24 @@ export function useBulkOperations({ newEdits[i] = { ...existingEdit, category }; }); setEditingFiles((/** @type {any} */ prev) => ({ ...prev, ...newEdits })); + // Save the choice the way a single-card edit does. The card edit state + // alone is dropped when Organize remounts or the file is re-analyzed. + const persistEdit = onEditChangeRef.current; + const persistBulkEdit = onBulkEditChangeRef.current; + if (typeof persistBulkEdit === 'function') { + // One call for the whole selection; a call per file costs a store + // update per file. + persistBulkEdit([...selected], 'category', category); + } else if (typeof persistEdit === 'function') { + selected.forEach((/** @type {any} */ key) => persistEdit(key, 'category', category)); + } setBulkEditMode(false); setBulkCategory(''); setSelectedFiles(new Set()); - notify(`Applied category "${category}" to ${selected.size} files`, 'success'); + notify( + `Applied category "${category}" to ${selected.size} ${selected.size === 1 ? 'file' : 'files'}`, + 'success' + ); }, 300); return () => { diff --git a/src/renderer/phases/organize/useOrganization.js b/src/renderer/phases/organize/useOrganization.js index bafe052..63f27b2 100644 --- a/src/renderer/phases/organize/useOrganization.js +++ b/src/renderer/phases/organize/useOrganization.js @@ -32,6 +32,16 @@ function describeFirstFailure(results) { return message; } +/** + * "1 file", "3 files". + * @param {number} count + * @param {string} [noun] + * @returns {string} + */ +function countOf(count, noun = 'file') { + return `${count} ${count === 1 ? noun : `${noun}s`}`; +} + const ROLLBACK_NAMES_SHOWN = 3; /** @@ -923,7 +933,7 @@ export function useOrganization({ // Reported once, after this block. } else if (result?.partialFailure || failedCount > 0) { addNotification( - `Organized ${uiResults.length} files with ${failedCount} failures. ${describeFirstFailure(resArray)}`, + `Organized ${countOf(uiResults.length)} with ${countOf(failedCount, 'failure')}. ${describeFirstFailure(resArray)}`, 'warning', 8000 ); @@ -933,7 +943,7 @@ export function useOrganization({ error: result?.error }); } else { - addNotification(`Organized ${uiResults.length} files`, 'success', 7000); + addNotification(`Organized ${countOf(uiResults.length)}`, 'success', 7000); } // Files moved on disk but the search index / history / undo @@ -963,11 +973,20 @@ export function useOrganization({ }); } - setBatchProgress({ - current: filesToProcess.length, - total: filesToProcess.length, - currentFile: '' - }); + // Only a run that got through every file fills the bar. A + // cancelled, paused, interrupted or rolled-back run keeps the + // count it reached, so it is not shown as done. + const stoppedEarly = + rolledBackBatch || + cancelledRef.current || + Boolean(result?.cancelled || result?.interrupted || result?.paused); + if (!stoppedEarly) { + setBatchProgress({ + current: filesToProcess.length, + total: filesToProcess.length, + currentFile: '' + }); + } } else if (!rolledBackBatch) { logger.warn('[ORGANIZE] onExecute: No successful results to process'); if (resArray.length > 0) { @@ -1060,12 +1079,14 @@ export function useOrganization({ if (failCount > 0) { addNotification( - `Undo partially complete: ${successCount} files restored, ${failCount} failed.`, + `Undo partially complete: ${countOf(successCount)} restored, ${failCount} failed.`, 'warning' ); } else if (successCount > 0) { addNotification( - `Undo complete. ${successCount} files restored to original locations.`, + successCount === 1 + ? 'Undo complete. 1 file restored to its original location.' + : `Undo complete. ${successCount} files restored to original locations.`, 'info' ); } @@ -1175,11 +1196,11 @@ export function useOrganization({ if (failCount > 0) { addNotification( - `Redo partially complete: ${successCount} files re-organized, ${failCount} failed.`, + `Redo partially complete: ${countOf(successCount)} re-organized, ${failCount} failed.`, 'warning' ); } else if (successCount > 0) { - addNotification(`Redo complete. ${successCount} files re-organized.`, 'info'); + addNotification(`Redo complete. ${countOf(successCount)} re-organized.`, 'info'); } } catch (redoError) { logger.error('[ORGANIZE] onRedo callback failed:', redoError.message); @@ -1193,7 +1214,7 @@ export function useOrganization({ const result = await executeAction( createOrganizeBatchAction( - `Organize ${operations.length} files`, + `Organize ${countOf(operations.length)}`, operations, stateCallbacks, resumeInfo ? { resumeBatchId: resumeInfo.batchId } : undefined diff --git a/src/renderer/phases/organize/useOrganizeNotificationFocus.js b/src/renderer/phases/organize/useOrganizeNotificationFocus.js index 790dd10..c68e611 100644 --- a/src/renderer/phases/organize/useOrganizeNotificationFocus.js +++ b/src/renderer/phases/organize/useOrganizeNotificationFocus.js @@ -31,6 +31,21 @@ export function useOrganizeNotificationFocus({ ); const handledRequestAtRef = useRef(0); + // The lookup below outlives the render that started it. Organize replaces + // the file lists right after mount (and whenever results stream in), so + // they are read through refs: a new list must not cancel a lookup that is + // still waiting on history or Explorer. + const unprocessedFilesRef = useRef(unprocessedFiles); + const processedFilesRef = useRef(processedFiles); + const addNotificationRef = useRef(addNotification); + const revealFileRef = useRef(revealFile); + useEffect(() => { + unprocessedFilesRef.current = unprocessedFiles; + processedFilesRef.current = processedFiles; + addNotificationRef.current = addNotification; + revealFileRef.current = revealFile; + }); + useEffect(() => { if (!organizeFocusRequest?.requestedAt) return; if (handledRequestAtRef.current === organizeFocusRequest.requestedAt) return; @@ -41,27 +56,35 @@ export function useOrganizeNotificationFocus({ const finish = () => dispatch(clearOrganizeFocus()); - const unprocessedIndex = findFileIndexByPathOrName(unprocessedFiles, filePath, fileName); - if (unprocessedIndex >= 0) { - setSelectedFiles(new Set([toCanonicalPath(unprocessedFiles[unprocessedIndex].path)])); - setFocusFileIndex(unprocessedIndex); - if (openDetails) { - setViewingFileDetails(unprocessedFiles[unprocessedIndex]); + /** Focus the file if it is in the queue or the organized list. */ + const focusInLists = () => { + const unprocessed = unprocessedFilesRef.current; + const unprocessedIndex = findFileIndexByPathOrName(unprocessed, filePath, fileName); + if (unprocessedIndex >= 0) { + setSelectedFiles(new Set([toCanonicalPath(unprocessed[unprocessedIndex].path)])); + setFocusFileIndex(unprocessedIndex); + if (openDetails) { + setViewingFileDetails(unprocessed[unprocessedIndex]); + } + return true; } - finish(); - return; - } - const processedMatch = findFileByPathOrName(processedFiles, filePath, fileName); - if (processedMatch) { - setShowHistoryModal(true); - if (openDetails) { - setViewingFileDetails({ - name: fileName || processedMatch.originalName || processedMatch.name, - path: filePath || processedMatch.path || processedMatch.currentPath, - analysis: processedMatch.analysis || null - }); + const processedMatch = findFileByPathOrName(processedFilesRef.current, filePath, fileName); + if (processedMatch) { + setShowHistoryModal(true); + if (openDetails) { + setViewingFileDetails({ + name: fileName || processedMatch.originalName || processedMatch.name, + path: filePath || processedMatch.path || processedMatch.currentPath, + analysis: processedMatch.analysis || null + }); + } + return true; } + return false; + }; + + if (focusInLists()) { finish(); return; } @@ -74,6 +97,10 @@ export function useOrganizeNotificationFocus({ const history = await window.electronAPI.analysisHistory.getFileHistory(filePath); if (cancelled) return; + // On a fresh mount the organized list is still empty when the + // request arrives; it is filled by the time history answers. + if (focusInLists()) return; + const entry = Array.isArray(history) ? history[0] : history; if (entry && typeof entry === 'object') { setViewingFileDetails({ @@ -81,46 +108,47 @@ export function useOrganizeNotificationFocus({ path: filePath, analysis: entry }); - finish(); return; } } - if (filePath && typeof revealFile === 'function') { - await revealFile(filePath); + const reveal = revealFileRef.current; + if (filePath && typeof reveal === 'function') { + await reveal(filePath); if (!cancelled) { - addNotification( + addNotificationRef.current( fileName ? `Opened ${fileName} in Explorer` : 'Opened file in Explorer', 'info', 3500 ); } } else if (fileName) { - addNotification(`Could not locate ${fileName} in Organize`, 'warning', 4500); + addNotificationRef.current(`Could not locate ${fileName} in Organize`, 'warning', 4500); } } catch (error) { logger.warn('Failed to focus file from notification', { error: error?.message, filePath }); if (!cancelled) { - addNotification('Could not open the file from this notification', 'warning', 4500); + addNotificationRef.current( + 'Could not open the file from this notification', + 'warning', + 4500 + ); } } finally { if (!cancelled) finish(); } })(); + // Runs on unmount or when a newer request replaces this one. return () => { cancelled = true; }; }, [ organizeFocusRequest, - unprocessedFiles, - processedFiles, setSelectedFiles, setFocusFileIndex, setViewingFileDetails, setShowHistoryModal, - addNotification, - revealFile, dispatch ]); } diff --git a/src/renderer/store/index.js b/src/renderer/store/index.js index 9335fc8..3e2031f 100644 --- a/src/renderer/store/index.js +++ b/src/renderer/store/index.js @@ -352,10 +352,12 @@ const loadState = () => { navigationError: null, lastOperationError: null, resetCounter: 0, - graphTourDismissed: false, + // "Don't show again" and the dock's collapsed state are durable + // preferences, not session state — keep them across TTL expiry. + graphTourDismissed: resolveDurableGraphTourDismissed(parsed?.ui?.graphTourDismissed), graphInsightsSection: 'insights', - graphLegendHintDismissed: false, - graphInsightsDockCollapsed: false, + graphLegendHintDismissed: parsed?.ui?.graphLegendHintDismissed === true, + graphInsightsDockCollapsed: parsed?.ui?.graphInsightsDockCollapsed === true, // Session-only flag: a fresh boot is never mid dashboard excursion. cameFromDashboard: false, // viewMode is a durable preference, not session state — preserve it @@ -426,7 +428,10 @@ const loadState = () => { _version: CURRENT_STATE_VERSION, ui: { viewMode: expiredState.ui.viewMode, - selectedTheme: expiredState.ui.selectedTheme + selectedTheme: expiredState.ui.selectedTheme, + graphTourDismissed: expiredState.ui.graphTourDismissed, + graphLegendHintDismissed: expiredState.ui.graphLegendHintDismissed, + graphInsightsDockCollapsed: expiredState.ui.graphInsightsDockCollapsed }, files: { selectedFiles: [], @@ -469,28 +474,7 @@ const loadState = () => { navigationError: null, lastOperationError: null, resetCounter: 0, - // "Don't show again" lives in two places by design: persisted Redux - // state (saved by the debounced persistence middleware) and a - // localStorage key that GraphTour writes *synchronously* on dismiss. - // The synchronous key is the durable fallback for the case where the - // debounced Redux save hasn't flushed before the app quits — so we - // read it but MUST NOT delete it here, or an immediate close right - // after dismissing would resurface the tour on next launch. `resetUi` - // already preserves this preference intentionally, so there is no - // "ghost after reset" to clean up; clearing is handled by the explicit - // re-show flow (unchecking the box) which removes both stores. - graphTourDismissed: (() => { - const fromState = parsed.ui?.graphTourDismissed === true; - let fromLegacy = false; - if (typeof localStorage !== 'undefined') { - try { - fromLegacy = localStorage.getItem('graphTourDismissed') === 'true'; - } catch { - /* private mode or quota — accept whatever we read */ - } - } - return fromState || fromLegacy; - })(), + graphTourDismissed: resolveDurableGraphTourDismissed(parsed.ui?.graphTourDismissed), graphInsightsSection: (() => { const v = parsed.ui?.graphInsightsSection; if (v == null) return 'insights'; @@ -561,6 +545,31 @@ function resolveDurableSelectedTheme(/** @type {any} */ parsedTheme) { return DURABLE_THEME_VALUES.includes(parsedTheme) ? parsedTheme : 'primary'; } +/** + * Resolve the graph tour's "Don't show again" flag to rehydrate. It lives in + * two places by design: persisted Redux state (saved by the debounced + * persistence middleware) and a localStorage key that GraphTour writes + * *synchronously* on dismiss. The synchronous key is the durable fallback for + * the case where the debounced Redux save hasn't flushed before the app quits — + * so we read it but MUST NOT delete it here, or an immediate close right after + * dismissing would resurface the tour on next launch. `resetUi` already + * preserves this preference intentionally, so there is no "ghost after reset" + * to clean up; clearing is handled by the explicit re-show flow (unchecking the + * box) which removes both stores. + */ +function resolveDurableGraphTourDismissed(/** @type {any} */ parsedFlag) { + const fromState = parsedFlag === true; + let fromLegacy = false; + if (typeof localStorage !== 'undefined') { + try { + fromLegacy = localStorage.getItem('graphTourDismissed') === 'true'; + } catch { + /* private mode or quota — accept whatever we read */ + } + } + return fromState || fromLegacy; +} + function createStratoSortStore() { const preloadedState = loadState(); return configureStore( @@ -664,10 +673,11 @@ if (isWebpackHmr && windowAny[__REDUX_DEV_HMR_KEY__]) { markStoreReady(); -// NOTE: State-expiration notification is handled by WelcomePhase.jsx (which -// shows a visible toast). The TTL-expired loadState path always resets to the -// WELCOME phase, so WelcomePhase is guaranteed to mount and display the notice. -// A previous duplicate handler here dispatched a silent Redux entry, causing -// two notification records for the same event. +// NOTE: State-expiration notification is handled by App.js (AppContent), which +// shows a visible toast. The TTL-expired loadState path keeps viewMode, so a +// dashboard user never mounts WelcomePhase at startup; the shell is mounted on +// both surfaces. Keep it the only consumer of the flag: a previous duplicate +// handler here dispatched a silent Redux entry, causing two notification +// records for the same event. export default store; diff --git a/src/renderer/store/slices/analysisSlice.js b/src/renderer/store/slices/analysisSlice.js index 810f1c7..ba43de5 100644 --- a/src/renderer/store/slices/analysisSlice.js +++ b/src/renderer/store/slices/analysisSlice.js @@ -166,6 +166,18 @@ const analysisSlice = createSlice({ }; } }, + // One pass for a bulk edit. A dispatch per file re-runs every selector + // over the whole list each time, which froze the window on large + // selections. + updateAnalysisResultsByPaths: (state, action) => { + const { paths, changes } = action.payload || {}; + if (!Array.isArray(paths) || paths.length === 0 || !changes) return; + const targets = new Set(paths.map((/** @type {any} */ p) => toCanonicalPath(p))); + state.results.forEach((/** @type {any} */ r) => { + if (!r || !targets.has(toCanonicalPath(r.path))) return; + r.analysis = { ...(r.analysis || {}), ...changes }; + }); + }, updateEmbeddingState: (state, action) => { const { path, embeddingPolicy, embeddingStatus } = action.payload || {}; if (!path) return; @@ -262,6 +274,7 @@ export const { setAnalysisResults, mergeAnalysisResults, updateAnalysisResult, + updateAnalysisResultsByPaths, updateEmbeddingState, resetAnalysisState, resetToSafeState, diff --git a/src/shared/atomicFileOperations.js b/src/shared/atomicFileOperations.js index 2cc3919..b979512 100644 --- a/src/shared/atomicFileOperations.js +++ b/src/shared/atomicFileOperations.js @@ -1800,13 +1800,23 @@ AtomicFileOperations.prototype.safeWriteFile = function safeWriteFile(filePath, * @param {Object} [options={}] - Options * @param {boolean} [options.verify=true] - Verify copy by comparing file sizes * @param {Function} [options.checksumFn] - Optional checksum function for additional verification + * @param {AbortSignal|null} [options.signal] - Caller's abort signal. A copy or + * checksum already running is not interrupted, but once the signal aborts + * nothing more is published or deleted: the source stays, the staging copy + * is removed and the call rejects with an AbortError. * @returns {Promise} Resolves when move is complete * @throws {Error} If copy fails or verification fails */ async function crossDeviceMove(source, dest, options = {}) { const normalizedSource = resolveFsPath(source); const normalizedDest = resolveFsPath(dest); - const { verify = true, checksumFn } = options; + const { verify = true, checksumFn, signal = null } = options; + const throwIfMoveAborted = () => { + if (!signal?.aborted) return; + const abortError = new Error(`Move aborted: ${normalizedSource}`); + abortError.name = 'AbortError'; + throw abortError; + }; // Stage the copy under a temp name IN THE DESTINATION DIRECTORY, then // rename into place. Copying straight to the canonical destination meant a @@ -1832,8 +1842,10 @@ async function crossDeviceMove(source, dest, options = {}) { await fs.mkdir(destinationDir, { recursive: true }); try { + throwIfMoveAborted(); // Copy to the staging name — never the canonical destination. await fs.copyFile(normalizedSource, stagingPath, fsConstants.COPYFILE_EXCL); + throwIfMoveAborted(); // Verify the staged copy before it can be published under the real name. if (verify) { @@ -1869,6 +1881,7 @@ async function crossDeviceMove(source, dest, options = {}) { // this rename is atomic. renameNoReplace preserves the no-overwrite // contract the old direct COPYFILE_EXCL copy provided — an occupied // destination surfaces as EEXIST instead of clobbering the occupant. + throwIfMoveAborted(); await renameNoReplace(stagingPath, normalizedDest); } catch (stagingError) { // Nothing was ever written at the canonical destination — only the @@ -1886,6 +1899,19 @@ async function crossDeviceMove(source, dest, options = {}) { throw stagingError; } + // An abort that landed with the publish: the source is still whole, so + // withdraw the copy just published instead of finishing a move the caller + // has already given up on (and may already have reported as not done). + if (signal?.aborted) { + await fs.unlink(normalizedDest).catch((withdrawError) => { + logger.error('[ATOMIC-OPS] Failed to withdraw published copy after abort', { + destination: normalizedDest, + error: withdrawError.message + }); + }); + throwIfMoveAborted(); + } + // Delete the source file — LAST, only after the verified copy is live at // the canonical destination. try { @@ -1926,7 +1952,7 @@ async function crossDeviceMove(source, dest, options = {}) { destination: normalizedDest }); } catch (error) { - if (error.isFileSystemError) { + if (error.isFileSystemError || error?.name === 'AbortError') { throw error; } // EEXIST can only come from the publish onto the destination name, so diff --git a/src/shared/ipcInvokeTimeout.js b/src/shared/ipcInvokeTimeout.js index 1a0682a..4e9fa0d 100644 --- a/src/shared/ipcInvokeTimeout.js +++ b/src/shared/ipcInvokeTimeout.js @@ -36,6 +36,20 @@ const VC_RUNTIME_INSTALL_INVOKE_TIMEOUT_MS = 11 * 60 * 1000; const ANALYZE_BATCH_INVOKE_TIMEOUT_MS = (TIMEOUTS.ANALYSIS_RUN_MAX || 6 * 60 * 60 * 1000) + 3 * (TIMEOUTS.GLOBAL_ANALYSIS || 600000); +/** + * FILES.PERFORM_OPERATION is one request for a whole organize batch (up to + * 1000 moves) or one move, copy or delete. A move to another drive copies and + * checksums the file, so neither has an upper bound. The 5-minute AI budget + * cut off batches that were still moving files: main was told to stop, the + * user saw a timeout, and the rest only moved at the next launch. The request + * therefore gets the longest delay a timer can hold (about 24.8 days; a + * longer one fires at once). Stop is what ends a batch early, and main gives + * up on a move that does not answer once the batch is stopped. A single + * move, copy or delete has no Stop: it ends when the operating system + * answers or fails the request. + */ +const FILE_OPERATION_INVOKE_TIMEOUT_MS = 2 ** 31 - 1; + /** * @param {string} channel * @returns {number} Timeout in milliseconds @@ -51,7 +65,6 @@ function getIpcInvokeTimeout(channel) { channel === IPC_CHANNELS.ANALYSIS.ANALYZE_IMAGE || channel === IPC_CHANNELS.ANALYSIS.ANALYZE_DOCUMENT || channel === IPC_CHANNELS.SUGGESTIONS.GET_FILE_SUGGESTIONS || - channel === IPC_CHANNELS.FILES.PERFORM_OPERATION || channel === IPC_CHANNELS.ORGANIZE.BATCH || channel === IPC_CHANNELS.SMART_FOLDERS.ADD || channel === IPC_CHANNELS.SMART_FOLDERS.GENERATE_DESCRIPTION || @@ -66,6 +79,9 @@ function getIpcInvokeTimeout(channel) { if (channel === IPC_CHANNELS.ANALYSIS.ANALYZE_BATCH) { timeout = ANALYZE_BATCH_INVOKE_TIMEOUT_MS; } + if (channel === IPC_CHANNELS.FILES.PERFORM_OPERATION) { + timeout = FILE_OPERATION_INVOKE_TIMEOUT_MS; + } // Embedding search/scoring operations involve embedding generation through the semaphore if ( channel === IPC_CHANNELS.EMBEDDINGS.SEARCH || @@ -128,6 +144,7 @@ function getIpcInvokeTimeout(channel) { module.exports = { ANALYZE_BATCH_INVOKE_TIMEOUT_MS, + FILE_OPERATION_INVOKE_TIMEOUT_MS, VC_RUNTIME_INSTALL_INVOKE_TIMEOUT_MS, getIpcInvokeTimeout }; diff --git a/test/AnalysisHistoryServiceCore.test.js b/test/AnalysisHistoryServiceCore.test.js index eca9503..f77fa32 100644 --- a/test/AnalysisHistoryServiceCore.test.js +++ b/test/AnalysisHistoryServiceCore.test.js @@ -728,6 +728,50 @@ describe('AnalysisHistoryServiceCore', () => { }); }); + describe('path-keyed writes use the current path of a moved entry', () => { + const movedEntry = () => ({ + id: 'a', + fileHash: 'h1', + timestamp: '2024-01-15T10:00:00Z', + originalPath: '/downloads/file.pdf', + fileName: 'file.pdf', + fileSize: 1, + analysis: { tags: [], category: 'docs' }, + organization: { actual: '/docs/file.pdf' } + }); + + beforeEach(async () => { + await service.initialize(); + service.analysisHistory.entries = { a: movedEntry() }; + }); + + test('removeEntriesByPath ignores the path the file left', async () => { + expect(await service.removeEntriesByPath('/downloads/file.pdf')).toEqual({ removed: 0 }); + expect(Object.keys(service.analysisHistory.entries)).toEqual(['a']); + + expect(await service.removeEntriesByPath('/docs/file.pdf')).toEqual({ removed: 1 }); + }); + + test('updateEntryPaths does not move it on behalf of a file at the old path', async () => { + const result = await service.updateEntryPaths([ + { oldPath: '/downloads/file.pdf', newPath: '/photos/other.pdf', newName: 'other.pdf' } + ]); + + expect(result).toMatchObject({ updated: 0, notFound: 1 }); + expect(service.analysisHistory.entries.a).toEqual(movedEntry()); + }); + + test('updateTags and updateEmbeddingStateByPath miss on the old path', async () => { + expect(await service.updateTags('/downloads/file.pdf', ['x'])).toMatchObject({ updated: 0 }); + expect( + await service.updateEmbeddingStateByPath('/downloads/file.pdf', { status: 'done' }) + ).toMatchObject({ updated: 0 }); + expect(service.analysisHistory.entries.a).toEqual(movedEntry()); + + expect(await service.updateTags('/docs/file.pdf', ['x'])).toMatchObject({ updated: 1 }); + }); + }); + describe('getAnalysisByPath', () => { beforeEach(async () => { await service.initialize(); diff --git a/test/ChatHistoryStore.validation.test.js b/test/ChatHistoryStore.validation.test.js index 27f58f8..681e263 100644 --- a/test/ChatHistoryStore.validation.test.js +++ b/test/ChatHistoryStore.validation.test.js @@ -46,7 +46,7 @@ describe('ChatHistoryStore — input size validation', () => { test('exposes finite numeric limits', () => { expect(CHAT_HISTORY_LIMITS.MAX_CONVERSATION_TITLE).toBe(512); expect(CHAT_HISTORY_LIMITS.MAX_MESSAGE_TEXT).toBe(64_000); - expect(CHAT_HISTORY_LIMITS.MAX_DOCUMENT_SCOPE_ITEMS).toBe(200); + expect(CHAT_HISTORY_LIMITS.MAX_DOCUMENT_SCOPE_ITEMS).toBe(1000); expect(CHAT_HISTORY_LIMITS.MAX_MESSAGE_PAYLOAD_BYTES).toBe(256_000); }); @@ -63,6 +63,21 @@ describe('ChatHistoryStore — input size validation', () => { expect(stmt.run).toHaveBeenCalled(); }); + test('accepts the largest document scope a chat request may carry', () => { + // chatQuerySchema (validationSchemas.js) allows 1000 documentScopeItems. + // A lower limit here made the first save of such a chat throw, so the + // conversation was never stored. + const { store, stmt } = buildStore(); + const scope = Array.from({ length: 1000 }, (_, i) => ({ + id: `file:${i}`, + path: `/synthetic/docs/${i}.pdf`, + name: `${i}.pdf`, + type: 'file' + })); + expect(() => store.createConversation('Big scope', scope, 'big')).not.toThrow(); + expect(stmt.run).toHaveBeenCalled(); + }); + test('rejects title over MAX_CONVERSATION_TITLE chars', () => { const { store, stmt } = buildStore(); const huge = 'x'.repeat(CHAT_HISTORY_LIMITS.MAX_CONVERSATION_TITLE + 1); diff --git a/test/ChatService.sessions.test.js b/test/ChatService.sessions.test.js index e80e2bf..69fe99e 100644 --- a/test/ChatService.sessions.test.js +++ b/test/ChatService.sessions.test.js @@ -124,7 +124,15 @@ describe('ChatService session management', () => { test('_saveMemoryTurn skips the user row on regenerate (skipUserMessage)', async () => { const store = { initialize: jest.fn().mockResolvedValue(undefined), - getConversation: jest.fn(() => ({ id: 'regen', title: 'Existing' })), + // The skip applies only when the question row is really stored. + getConversation: jest.fn(() => ({ + id: 'regen', + title: 'Existing', + messages: [ + { role: 'user', text: 'same question again' }, + { role: 'assistant', text: 'first answer' } + ] + })), createConversation: jest.fn(), addMessage: jest.fn() }; diff --git a/test/DownloadWatcher.lifecycle.test.js b/test/DownloadWatcher.lifecycle.test.js index 2a380f6..86f5fb9 100644 --- a/test/DownloadWatcher.lifecycle.test.js +++ b/test/DownloadWatcher.lifecycle.test.js @@ -253,6 +253,32 @@ describe('DownloadWatcher lifecycle', () => { expect(watcher.watcher).toBeNull(); expect(watcher._stopped).toBe(false); }); + + test('start() during a pending stop() leaves a live watcher', async () => { + const watcher = makeWatcher(); + await startAndSettle(watcher); + + // stop() drains the download being processed before it closes chokidar. + let releasePipeline; + const pipeline = new Promise((resolve) => { + releasePipeline = resolve; + }); + watcher._inFlightHandles.add(pipeline); + + const stopping = watcher.stop(); + await flushMicrotasks(); + // Auto-organize is turned on again before the stop has finished. + const starting = watcher.start(); + await flushMicrotasks(); + + watcher._inFlightHandles.delete(pipeline); + releasePipeline(); + await flushMicrotasks(); + await Promise.all([stopping, starting]); + + expect(chokidar.watch).toHaveBeenCalledTimes(2); + expect(watcher.watcher).toBe(mockWatchers[1]); + }); }); describe('_handleFileDeletion gates', () => { diff --git a/test/EmbeddingQueueCore.test.js b/test/EmbeddingQueueCore.test.js index 6bca5f6..8c2d5b9 100644 --- a/test/EmbeddingQueueCore.test.js +++ b/test/EmbeddingQueueCore.test.js @@ -45,7 +45,11 @@ jest.mock('../src/shared/errorCodes', () => ({ jest.mock('../src/main/analysis/embeddingQueue/persistence', () => ({ loadPersistedData: jest.fn().mockResolvedValue(undefined), - persistQueueData: jest.fn().mockResolvedValue(undefined) + persistQueueData: jest.fn().mockResolvedValue(undefined), + // Used by the real failed-item handler in the retry-schedule tests below. + persistFailedItems: jest.fn().mockResolvedValue(true), + persistDeadLetterQueue: jest.fn().mockResolvedValue(true), + SQLITE_KEYS: {} })); const mockFailedHandler = { @@ -53,6 +57,7 @@ const mockFailedHandler = { deadLetterQueue: [], trackFailedItem: jest.fn(), retryFailedItems: jest.fn().mockResolvedValue(undefined), + getNextRetryDelay: jest.fn(() => null), setDeadLetterQueue: jest.fn(), persistPending: jest.fn().mockResolvedValue(undefined) }; @@ -947,4 +952,168 @@ describe('EmbeddingQueueCore', () => { mockFailedHandler.trackFailedItem.mockImplementation(() => undefined); } }); + + // A failed item used to be retried only from inside a later flush, and a + // flush with an empty queue returns early. So a failure in the last batch + // (or a database-offline give-up) stayed in the failed-items map, across + // restarts, until some other file was enqueued. + describe('failed items are retried on their own schedule', () => { + const { + createFailedItemHandler + } = require('../src/main/analysis/embeddingQueue/failedItemHandler'); + const { + processItemsInParallel + } = require('../src/main/analysis/embeddingQueue/parallelProcessor'); + const { loadPersistedData } = require('../src/main/analysis/embeddingQueue/persistence'); + const realHandlerModule = jest.requireActual( + '../src/main/analysis/embeddingQueue/failedItemHandler' + ); + + const succeedAll = async ({ items, startProcessedCount }) => startProcessedCount + items.length; + const failAll = async ({ items, failedItemIds, startProcessedCount, onItemFailed }) => { + for (const item of items) { + failedItemIds.add(item.id); + onItemFailed(item, 'upsert timed out'); + } + return startProcessedCount; + }; + const flushedIds = () => + processItemsInParallel.mock.calls.map(([args]) => args.items.map((item) => item.id)); + + const createQueue = () => { + // The real handler (mocked persistence, 10 ms mocked backoff base). + createFailedItemHandler.mockImplementationOnce(realHandlerModule.createFailedItemHandler); + const EmbeddingQueue = require('../src/main/analysis/embeddingQueue/EmbeddingQueueCore'); + const q = new EmbeddingQueue({ flushDelayMs: 0 }); + q.MAX_QUEUE_SIZE = 100; + return q; + }; + + beforeEach(() => { + jest.useFakeTimers(); + processItemsInParallel.mockReset(); + processItemsInParallel.mockImplementation(succeedAll); + mockContainer.resolve.mockReset(); + mockContainer.resolve.mockReturnValue({ + initialize: jest.fn().mockResolvedValue(undefined), + isOnline: true + }); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + processItemsInParallel.mockReset(); + processItemsInParallel.mockImplementation(succeedAll); + mockContainer.resolve.mockReset(); + loadPersistedData.mockImplementation(async () => undefined); + }); + + test('an item that failed in the last batch is embedded later with nothing else enqueued', async () => { + processItemsInParallel.mockImplementationOnce(failAll); + const q = createQueue(); + q.initialized = true; + q.queue = [{ id: 'file:C:\\late.pdf', vector: [1] }]; + + await q.flush(); + expect(q.queue).toHaveLength(0); + expect(q._failedItemHandler.failedItems.has('file:C:\\late.pdf')).toBe(true); + + await advance(5000); + + expect(flushedIds()).toEqual([['file:C:\\late.pdf'], ['file:C:\\late.pdf']]); + expect(q._failedItemHandler.failedItems.size).toBe(0); + expect(q.queue).toHaveLength(0); + expect(jest.getTimerCount()).toBe(0); + }); + + test('failed items restored at startup are retried without a new enqueue', async () => { + loadPersistedData.mockImplementation(async (_path, onLoad, label) => { + if (label === 'failed items') { + onLoad([ + [ + 'file:C:\\restored.pdf', + { + item: { id: 'file:C:\\restored.pdf', vector: [1] }, + retryCount: 1, + lastAttempt: 0, + error: 'upsert timed out' + } + ] + ]); + } + }); + const q = createQueue(); + + await q.initialize(); + expect(q._failedItemHandler.failedItems.size).toBe(1); + + await advance(5000); + + expect(flushedIds()).toEqual([['file:C:\\restored.pdf']]); + expect(q._failedItemHandler.failedItems.size).toBe(0); + }); + + test('a batch given up on while the database was offline is embedded once it is back', async () => { + const online = { initialize: jest.fn().mockResolvedValue(undefined), isOnline: true }; + mockContainer.resolve.mockImplementationOnce(() => { + throw new Error('no service'); + }); + mockContainer.resolve.mockReturnValue(online); + const q = createQueue(); + q.initialized = true; + q.MAX_RETRY_COUNT = 1; + q.queue = [{ id: 'file:C:\\offline.pdf', vector: [1] }]; + + await q.flush(); + expect(q.queue).toHaveLength(0); + expect(q._failedItemHandler.failedItems.size).toBe(1); + expect(processItemsInParallel).not.toHaveBeenCalled(); + + await advance(5000); + + expect(flushedIds()).toEqual([['file:C:\\offline.pdf']]); + expect(q._failedItemHandler.failedItems.size).toBe(0); + }); + + test('an item that keeps failing stops at the retry limit with the backoff between attempts', async () => { + processItemsInParallel.mockImplementation(failAll); + const q = createQueue(); + q.initialized = true; + q.queue = [{ id: 'file:C:\\bad.pdf', vector: [1] }]; + + await q.flush(); + expect(processItemsInParallel).toHaveBeenCalledTimes(1); + // Not retried before its backoff has elapsed. + await advance(5); + expect(processItemsInParallel).toHaveBeenCalledTimes(1); + + await advance(60000); + + // First attempt + 3 retries (ANALYSIS.retryAttempts), then dead-lettered. + expect(processItemsInParallel).toHaveBeenCalledTimes(4); + expect(q._failedItemHandler.failedItems.size).toBe(0); + expect(q._failedItemHandler.deadLetterQueue).toHaveLength(1); + expect(jest.getTimerCount()).toBe(0); + }); + + test('shutdown clears the pending retry so nothing runs or stays armed afterwards', async () => { + processItemsInParallel.mockImplementationOnce(failAll); + const q = createQueue(); + q.initialized = true; + q.queue = [{ id: 'file:C:\\late.pdf', vector: [1] }]; + + await q.flush(); + expect(q._failedRetryTimer).toBeTruthy(); + + await q.shutdown(); + expect(q._failedRetryTimer).toBeNull(); + expect(jest.getTimerCount()).toBe(0); + + await advance(60000); + expect(processItemsInParallel).toHaveBeenCalledTimes(1); + // Still recorded for the next start. + expect(q._failedItemHandler.failedItems.size).toBe(1); + }); + }); }); diff --git a/test/ModelDownloadManager.test.js b/test/ModelDownloadManager.test.js index 1209772..243a2e1 100644 --- a/test/ModelDownloadManager.test.js +++ b/test/ModelDownloadManager.test.js @@ -559,6 +559,139 @@ describe('ModelDownloadManager', () => { expect(manager.isDownloading('alpha.gguf')).toBe(false); }); + describe('connection closed before the body is complete', () => { + // A clean early close (FIN before Content-Length is satisfied) makes Node + // emit 'aborted' and 'close' on the response, with no 'end' and no error + // on the request, so the piped write stream never finishes. + const makeWriteStream = () => { + const ws = new EventEmitter(); + ws.destroy = jest.fn(() => { + ws.destroyed = true; + setTimeout(() => ws.emit('close'), 0); + }); + return ws; + }; + const makeRequest = () => { + const request = new EventEmitter(); + request.setTimeout = jest.fn(); + request.destroy = jest.fn(); + return request; + }; + const makeTruncatedResponse = () => { + const response = new EventEmitter(); + response.statusCode = 200; + response.statusMessage = 'OK'; + response.headers = { 'content-length': '2048' }; + response.complete = false; + response.resume = jest.fn(); + response.unpipe = jest.fn(); + response.pipe = (ws) => { + setTimeout(() => { + response.emit('data', Buffer.alloc(1000)); + response.emit('aborted'); + response.emit('close'); + }, 0); + return ws; + }; + return response; + }; + + afterEach(() => { + jest.useRealTimers(); + }); + + test('downloadModel fails instead of hanging and keeps the partial for resume', async () => { + jest.useFakeTimers(); + const manager = new ModelDownloadManager(); + const writeStream = makeWriteStream(); + fsModule.createWriteStream.mockReturnValue(writeStream); + mockFs.stat.mockRejectedValueOnce(new Error('no partial')); + + https.get.mockImplementation((_options, onResponse) => { + setTimeout(() => onResponse(makeTruncatedResponse()), 0); + return makeRequest(); + }); + + /** @type {any} */ + let outcome = null; + manager.downloadModel('alpha.gguf', { _maxRetries: 0 }).then( + (value) => { + outcome = { resolved: value }; + }, + (error) => { + outcome = { rejected: error }; + } + ); + await jest.advanceTimersByTimeAsync(100); + + expect(outcome?.rejected?.code).toBe('ECONNRESET'); + expect(writeStream.destroy).toHaveBeenCalled(); + expect(manager.isDownloading('alpha.gguf')).toBe(false); + expect(manager._downloads.has('alpha.gguf')).toBe(false); + expect(mockFs.unlink).not.toHaveBeenCalled(); + }); + + test('downloadModel retries with a Range request from the partial', async () => { + jest.useFakeTimers(); + const manager = new ModelDownloadManager(); + fsModule.createWriteStream.mockImplementation(makeWriteStream); + + // Attempt 1: no partial. Attempt 2: 1000 bytes on disk, then full size. + mockFs.stat + .mockRejectedValueOnce(new Error('no partial')) + .mockResolvedValueOnce({ size: 1000 }) + .mockResolvedValueOnce({ size: 2048 }); + + const requestOptionsSeen = []; + https.get.mockImplementation((requestOptions, onResponse) => { + requestOptionsSeen.push(requestOptions); + if (requestOptionsSeen.length === 1) { + setTimeout(() => onResponse(makeTruncatedResponse()), 0); + return makeRequest(); + } + const response = new EventEmitter(); + response.statusCode = 206; + response.statusMessage = 'Partial Content'; + response.headers = { 'content-length': '1048' }; + response.resume = jest.fn(); + response.pipe = (ws) => { + setTimeout(() => { + response.complete = true; + ws.emit('finish'); + response.emit('close'); + }, 0); + return ws; + }; + setTimeout(() => onResponse(response), 0); + return makeRequest(); + }); + + /** @type {any} */ + let outcome = null; + manager.downloadModel('alpha.gguf').then( + (value) => { + outcome = { resolved: value }; + }, + (error) => { + outcome = { rejected: error }; + } + ); + await jest.advanceTimersByTimeAsync(5000); + + expect(outcome?.resolved).toEqual(expect.objectContaining({ success: true })); + expect(requestOptionsSeen).toHaveLength(2); + expect(requestOptionsSeen[0].headers.Range).toBeUndefined(); + expect(requestOptionsSeen[1].headers.Range).toBe('bytes=1000-'); + expect(fsModule.createWriteStream).toHaveBeenLastCalledWith( + expect.stringContaining('alpha.gguf.partial'), + { flags: 'a' } + ); + expect(mockFs.unlink).not.toHaveBeenCalled(); + expect(mockFs.rename).toHaveBeenCalledTimes(1); + expect(manager.isDownloading('alpha.gguf')).toBe(false); + }); + }); + describe('_wrapWriteStreamError', () => { test('maps ENOSPC to "Disk full" message and preserves code + cause', () => { const manager = new ModelDownloadManager(); diff --git a/test/OramaVectorService.test.js b/test/OramaVectorService.test.js index 9eef149..1c1166a 100644 --- a/test/OramaVectorService.test.js +++ b/test/OramaVectorService.test.js @@ -503,6 +503,118 @@ describe('OramaVectorService', () => { expect(file.hasVector).toBe(false); expect(service._embeddingStore.files.has('novec-id')).toBe(false); }); + + // Regression: a row left behind at the destination id (a file deleted outside + // the app keeps its row, orphan-marked, for up to 30 days) made Orama's insert + // throw DOCUMENT_ALREADY_EXISTS. The re-key was swallowed and the moved file + // ended up served by the deleted file's vector, hidden by its orphan flag. + describe('destination id already holds a row', () => { + const staleVector = new Array(768).fill(0.9); + const movedVector = new Array(768).fill(0).map((_, i) => (i % 2 === 0 ? 0.5 : -0.5)); + const staleId = 'file:C:\\dest\\report.pdf'; + const movedId = 'file:C:\\inbox\\report.pdf'; + + const seed = async () => { + await service.upsertFile({ + id: staleId, + vector: staleVector, + meta: { + path: 'C:\\dest\\report.pdf', + fileName: 'report.pdf', + fileType: 'application/pdf', + suggestedName: 'deleted-file' + } + }); + await service.upsertFile({ + id: movedId, + vector: movedVector, + meta: { + path: 'C:\\inbox\\report.pdf', + fileName: 'report.pdf', + fileType: 'application/pdf', + suggestedName: 'moved-file' + } + }); + }; + + const chunksFor = (fileId, n, vector) => + Array.from({ length: n }, (_, i) => ({ + id: `chunk:${fileId}:${i}`, + vector, + meta: { fileId, chunkIndex: i, content: `passage ${i} of ${fileId}` }, + document: `passage ${i}` + })); + + test('updateFilePaths replaces a stale orphaned row at the destination id', async () => { + await seed(); + await service.markEmbeddingsOrphaned([staleId]); + + const updated = await service.updateFilePaths([ + { oldId: movedId, newId: staleId, newMeta: { path: 'C:\\dest\\report.pdf' } } + ]); + + expect(updated).toBe(1); + expect(mockLogger.warn).not.toHaveBeenCalledWith( + '[OramaVectorService] Failed to update file path:', + expect.anything() + ); + await expect(service.getFile(movedId)).resolves.toBeNull(); + + const file = await service.getFile(staleId); + expect(file.suggestedName).toBe('moved-file'); + expect(file.isOrphaned).toBe(false); + expect(file.orphanedAt).toBe(''); + expect(service._embeddingStore.files.get(staleId)).toEqual(movedVector); + expect(service._embeddingStore.files.has(movedId)).toBe(false); + + const results = await service.querySimilarFiles(movedVector, 5); + expect(results.some((r) => r.id === staleId)).toBe(true); + }); + + test('updateFilePaths clears the orphan flag the moved row carried', async () => { + await seed(); + await service.markEmbeddingsOrphaned([movedId]); + + const updated = await service.updateFilePaths([ + { oldId: movedId, newId: 'file:C:\\elsewhere\\report.pdf' } + ]); + + expect(updated).toBe(1); + const file = await service.getFile('file:C:\\elsewhere\\report.pdf'); + expect(file.isOrphaned).toBe(false); + expect(file.orphanedAt).toBe(''); + }); + + test('updateFilePaths drops the stale chunks of the replaced destination row', async () => { + await seed(); + await service.batchUpsertFileChunks(chunksFor(staleId, 3, staleVector)); + await service.batchUpsertFileChunks(chunksFor(movedId, 1, movedVector)); + + await service.updateFilePaths([{ oldId: movedId, newId: staleId }]); + // FilePathCoordinator clones the moved file's chunks after the re-key. + await service.cloneFileChunks(movedId, staleId); + + const chunks = await service.getChunksForFile(staleId); + expect(chunks.map((c) => c.text)).toEqual([`passage 0 of ${movedId}`]); + }); + + test('updateFilePaths keeps a destination row that the same batch still has to move', async () => { + await seed(); + const onwardId = 'file:C:\\archive\\report.pdf'; + + const updated = await service.updateFilePaths([ + { oldId: movedId, newId: staleId }, + { oldId: staleId, newId: onwardId } + ]); + + // The first spec fails as it always did; the row it would have replaced + // belongs to a live file and reaches its own new id intact. + expect(updated).toBe(1); + expect((await service.getFile(onwardId)).suggestedName).toBe('deleted-file'); + expect(service._embeddingStore.files.get(onwardId)).toEqual(staleVector); + expect((await service.getFile(movedId)).suggestedName).toBe('moved-file'); + }); + }); }); describe('persistence scheduling', () => { diff --git a/test/OrganizationSuggestionService.test.js b/test/OrganizationSuggestionService.test.js index 9b63b09..dea5624 100644 --- a/test/OrganizationSuggestionService.test.js +++ b/test/OrganizationSuggestionService.test.js @@ -655,6 +655,86 @@ describe('OrganizationSuggestionService', () => { const adjusted = await service.applyFeedbackMemoryAdjustments(file, suggestions); expect(adjusted[0].folder).toBe('CAD'); }); + + test('an unrelated memory leaves confidence and score as they were', async () => { + const file = createTestFile(); + await service.addFeedbackMemory('Keep folder names short', { source: 'manual' }); + + // weightedScore is the ranking key: score x the semantic source weight (1.1). + const suggestions = [ + { folder: 'Invoices', source: 'semantic', score: 0.69, confidence: 0.69 }, + { folder: 'Projects', source: 'llm', score: 0.55, confidence: 0.55 } + ].map((s) => ({ ...s, weightedScore: s.score * (s.source === 'llm' ? 1.5 : 1.1) })); + + const adjusted = await service.applyFeedbackMemoryAdjustments(file, suggestions); + + const invoices = adjusted.find((s) => s.folder === 'Invoices'); + const projects = adjusted.find((s) => s.folder === 'Projects'); + expect(invoices.confidence).toBe(0.69); + expect(invoices.score).toBe(0.69); + expect(invoices.memoryAdjustment).toBeUndefined(); + expect(projects.confidence).toBe(0.55); + expect(projects.score).toBe(0.55); + // Ranking still follows the weighted score: llm 0.825 beats semantic 0.759. + expect(adjusted[0].folder).toBe('Projects'); + }); + + test('a matching memory raises confidence from the confidence, not from the ranking score', async () => { + const file = createTestFile(); + const memory = await service.addFeedbackMemory('Invoices go to Finance', { + source: 'manual' + }); + mockVectorDbService.queryFeedbackMemory.mockResolvedValue([ + { id: memory.id, score: 1, metadata: {}, document: memory.text } + ]); + + const suggestions = [ + { folder: 'Finance', source: 'llm', score: 0.5, confidence: 0.5, weightedScore: 0.75 }, + { folder: 'Projects', source: 'llm', score: 0.8, confidence: 0.8, weightedScore: 1.2 } + ]; + + const adjusted = await service.applyFeedbackMemoryAdjustments(file, suggestions); + + const finance = adjusted.find((s) => s.folder === 'Finance'); + expect(finance.memoryAdjustment).toEqual({ boost: 0.15, penalty: 0 }); + expect(finance.confidence).toBeCloseTo(0.65, 10); + expect(finance.score).toBeCloseTo(0.65, 10); + expect(finance.weightedScore).toBeCloseTo(0.9, 10); + // A weighted score above 1 keeps its lead; clamping it would tie the two. + expect(adjusted[0].folder).toBe('Projects'); + }); + + test('getSuggestionsForFile returns the same confidence with and without an unrelated memory', async () => { + const file = createTestFile(); + const smartFolders = createTestSmartFolders(3); + mockVectorDbService.queryFolders.mockResolvedValue([ + { + id: 'folder-1', + score: 0.69, + metadata: { path: '/docs/Invoices', folderName: 'Invoices' } + } + ]); + // mockLlama is shared by the whole file and clearAllMocks keeps + // implementations, so put back what the tests after this one rely on. + const previousGenerateText = mockLlama.generateText.getMockImplementation(); + mockLlama.generateText.mockResolvedValue({ + response: JSON.stringify({ suggestions: [] }) + }); + + const before = await service.getSuggestionsForFile(file, smartFolders); + expect(before.success).toBe(true); + expect(before.primary.folder).toBe('Invoices'); + expect(before.confidence).toBe(0.69); + + await service.addFeedbackMemory('Keep folder names short', { source: 'manual' }); + + const after = await service.getSuggestionsForFile(file, smartFolders); + expect(after.primary.folder).toBe('Invoices'); + expect(after.confidence).toBe(before.confidence); + // Below the 0.75 auto-organize default: the file stays held for review. + expect(after.confidence).toBeLessThan(0.75); + mockLlama.generateText.mockImplementation(previousGenerateText); + }); }); describe('Error Handling and Edge Cases', () => { diff --git a/test/SessionMemory.test.js b/test/SessionMemory.test.js index e2819ec..3d07e79 100644 --- a/test/SessionMemory.test.js +++ b/test/SessionMemory.test.js @@ -95,6 +95,94 @@ describe('SessionMemory', () => { expect(history).not.toContain('[doc-1]'); }); + /** In-memory stand-in for ChatHistoryStore: conversations keyed by id. */ + function createFakeStore({ failCreate = false } = {}) { + /** @type {Map} */ + const conversations = new Map(); + return { + conversations, + getConversation: jest.fn((id) => conversations.get(id) || null), + createConversation: jest.fn((title, documentScope, id) => { + if (failCreate) throw new Error('Document scope exceeds limit'); + conversations.set(id, { id, title, documentScope, messages: [] }); + }), + addMessage: jest.fn((id, message) => { + conversations.get(id).messages.push(message); + }) + }; + } + + test('retry after a failed first attempt saves the question with the answer', async () => { + // The failed attempt persisted nothing, so there is no user row to reuse. + const store = createFakeStore(); + const memory = new SessionMemory({ chatHistoryStore: store }); + const session = await memory.getSessionMemory('retry-new'); + + await memory.saveMemoryTurn( + session, + 'What is in the report?', + { text: 'A summary.' }, + { skipUserMessage: true } + ); + + expect(store.conversations.get('retry-new').messages.map((m) => [m.role, m.text])).toEqual([ + ['user', 'What is in the report?'], + ['assistant', 'A summary.'] + ]); + }); + + test('retry of a failed follow-up saves the follow-up question', async () => { + const store = createFakeStore(); + const memory = new SessionMemory({ chatHistoryStore: store }); + const session = await memory.getSessionMemory('retry-followup'); + await memory.saveMemoryTurn(session, 'First question', { text: 'First answer' }); + + await memory.saveMemoryTurn( + session, + 'And the second one?', + { text: 'Second answer' }, + { skipUserMessage: true } + ); + + const rows = store.conversations.get('retry-followup').messages.map((m) => [m.role, m.text]); + expect(rows).toEqual([ + ['user', 'First question'], + ['assistant', 'First answer'], + ['user', 'And the second one?'], + ['assistant', 'Second answer'] + ]); + expect(await memory.getHistoryText(session)).toContain('User: And the second one?'); + }); + + test('regenerate does not duplicate a question that is already saved', async () => { + const store = createFakeStore(); + const memory = new SessionMemory({ chatHistoryStore: store }); + const session = await memory.getSessionMemory('regen'); + await memory.saveMemoryTurn(session, 'Same question', { text: 'First answer' }); + + await memory.saveMemoryTurn( + session, + 'Same question', + { text: 'Regenerated answer' }, + { skipUserMessage: true } + ); + + const roles = store.conversations.get('regen').messages.map((m) => m.role); + expect(roles).toEqual(['user', 'assistant', 'assistant']); + }); + + test('follow-ups keep their history when a turn could not be saved', async () => { + const store = createFakeStore({ failCreate: true }); + const memory = new SessionMemory({ chatHistoryStore: store }); + const session = await memory.getSessionMemory('unsaved'); + + await memory.saveMemoryTurn(session, 'First question', { text: 'First answer' }); + + const history = await memory.getHistoryText(session); + expect(history).toContain('User: First question'); + expect(history).toContain('Assistant: First answer'); + }); + test('delete removes the session from the LRU', async () => { const memory = new SessionMemory(); await memory.getSessionMemory('gone'); diff --git a/test/SmartFolderWatcher.coverage.test.js b/test/SmartFolderWatcher.coverage.test.js index 933289e..9a9fe1f 100644 --- a/test/SmartFolderWatcher.coverage.test.js +++ b/test/SmartFolderWatcher.coverage.test.js @@ -191,6 +191,8 @@ const fs = require('fs').promises; const chokidar = require('chokidar'); const SmartFolderWatcher = require('../src/main/services/SmartFolderWatcher'); const { recordAnalysisResult } = require('../src/main/ipc/analysisUtils'); +const { getInstance: getFileOperationTracker } = require('../src/shared/fileOperationTracker'); +const { deferGhostCleanup, resumeGhostCleanup } = require('../src/shared/ghostCleanupGuard'); describe('SmartFolderWatcher', () => { let watcher; @@ -444,6 +446,177 @@ describe('SmartFolderWatcher', () => { expect(watcher._watchToggleWork).toBeUndefined(); expect(watcher.isRunning).toBe(true); }); + + describe('re-enabling while stop() is still running', () => { + /** Holds the next stop() on its queue flush until the returned function is called. */ + const holdStop = () => { + let release; + watcher._flushQueuePersist = jest + .fn() + .mockResolvedValue(undefined) + .mockImplementationOnce( + () => + new Promise((resolve) => { + release = resolve; + }) + ); + return () => release(); + }; + + test('the watcher is running again once the stop has finished', async () => { + await watcher.start(); + const releaseStop = holdStop(); + + watcher.applySettings({ smartFolderWatchEnabled: false }); + const stopWork = watcher._watchToggleWork; + expect(watcher._isStopping).toBe(true); + + watcher.applySettings({ smartFolderWatchEnabled: true }); + const startWork = watcher._watchToggleWork; + + releaseStop(); + await stopWork; + await startWork; + + expect(watcher.isRunning).toBe(true); + expect(watcher.watcher).not.toBeNull(); + }); + + test('start() waits for the stop instead of reporting the dying watcher as running', async () => { + await watcher.start(); + const dyingWatcher = watcher.watcher; + const releaseStop = holdStop(); + + const stopWork = watcher.stop(); + const startWork = watcher.start(); + + releaseStop(); + await stopWork; + await expect(startWork).resolves.toBe(true); + + expect(watcher.isRunning).toBe(true); + expect(watcher.watcher).not.toBeNull(); + expect(watcher.watcher).not.toBe(dyingWatcher); + }); + + test('a disable saved after the re-enable still wins', async () => { + await watcher.start(); + const releaseStop = holdStop(); + + watcher.applySettings({ smartFolderWatchEnabled: false }); + const stopWork = watcher._watchToggleWork; + watcher.applySettings({ smartFolderWatchEnabled: true }); + const startWork = watcher._watchToggleWork; + // The user turns it off again before the stop has finished. + mockDeps.settingsService.load.mockResolvedValue({ smartFolderWatchEnabled: false }); + + releaseStop(); + await stopWork; + await startWork; + + expect(watcher.isRunning).toBe(false); + expect(watcher.watcher).toBeNull(); + }); + }); + }); + + describe('_handleFileDeletion: a move made by the app is not a deletion', () => { + const movedOut = '/docs/moved-by-app.pdf'; + + beforeEach(() => { + mockDeps.analysisHistoryService.getAnalysisByPath = jest + .fn() + .mockResolvedValue({ fileSize: 100, lastModified: 1000 }); + mockDeps.analysisHistoryService.removeEntriesByPath = jest.fn().mockResolvedValue(); + }); + + test('an unlink of a path the app just moved keeps its history and vectors', async () => { + getFileOperationTracker.mockReturnValueOnce({ + wasRecentlyOperated: jest.fn((p) => p === movedOut), + recordOperation: jest.fn() + }); + const finalize = jest.spyOn(watcher, '_finalizeDeletion'); + + await watcher._handleFileDeletion(movedOut); + + expect(watcher._pendingMoveCandidates.size).toBe(0); + expect(finalize).not.toHaveBeenCalled(); + expect(mockDeps.analysisHistoryService.removeEntriesByPath).not.toHaveBeenCalled(); + }); + + test('an unlink during a batch organize keeps its history and vectors', async () => { + const finalize = jest.spyOn(watcher, '_finalizeDeletion'); + + deferGhostCleanup(); + try { + await watcher._handleFileDeletion(movedOut); + } finally { + resumeGhostCleanup(); + } + + expect(watcher._pendingMoveCandidates.size).toBe(0); + expect(finalize).not.toHaveBeenCalled(); + expect(mockDeps.analysisHistoryService.removeEntriesByPath).not.toHaveBeenCalled(); + }); + + test('a deletion made outside the app is still cleaned up', async () => { + // No history entry to match a move against, so cleanup runs at once. + mockDeps.analysisHistoryService.getAnalysisByPath.mockResolvedValue(null); + + await watcher._handleFileDeletion('/docs/deleted-in-explorer.pdf'); + + expect(mockDeps.analysisHistoryService.removeEntriesByPath).toHaveBeenCalledWith( + '/docs/deleted-in-explorer.pdf' + ); + }); + }); + + describe('move detection window', () => { + test('outlasts the wait before the matching add is looked at', () => { + // chokidar holds an add for stabilityThreshold; the watcher then + // debounces it before it looks for a pending deletion to pair it with. + expect(watcher._moveDetectionWindowMs).toBeGreaterThan( + watcher.stabilityThreshold + watcher.debounceDelay + ); + }); + + test('a rename in Explorer moves the analysis instead of deleting it', async () => { + const oldPath = '/docs/report.pdf'; + const newPath = '/docs/report-final.pdf'; + mockDeps.analysisHistoryService.getAnalysisByPath = jest + .fn() + .mockResolvedValue({ fileSize: 100, lastModified: 1000 }); + mockDeps.analysisHistoryService.removeEntriesByPath = jest.fn().mockResolvedValue(); + const atomicPathUpdate = jest.fn().mockResolvedValue({ success: true }); + watcher.filePathCoordinator = { atomicPathUpdate }; + fs.access.mockResolvedValue(undefined); + + jest.useFakeTimers({ doNotFake: ['nextTick'] }); + try { + // chokidar publishes the unlink at once... + await watcher._handleFileDeletion(oldPath); + expect(watcher._pendingMoveCandidates.size).toBe(1); + + // ...and the add only after the file was stable for stabilityThreshold + // (checked on a 200 ms poll). + await jest.advanceTimersByTimeAsync(watcher.stabilityThreshold + 200); + watcher._handleFileEvent('add', newPath, { + size: 100, + mtimeMs: 1000, + mtime: new Date(1000) + }); + await jest.advanceTimersByTimeAsync(watcher.debounceDelay); + for (let i = 0; i < 5; i++) { + await jest.advanceTimersByTimeAsync(0); + } + + expect(atomicPathUpdate).toHaveBeenCalledWith(oldPath, newPath, { type: 'move' }); + expect(mockDeps.analysisHistoryService.removeEntriesByPath).not.toHaveBeenCalled(); + expect(watcher._pendingMoveCandidates.size).toBe(0); + } finally { + jest.useRealTimers(); + } + }); }); describe('updateWatchedFolders', () => { diff --git a/test/UndoRedoService.test.js b/test/UndoRedoService.test.js index c621033..d334a0d 100644 --- a/test/UndoRedoService.test.js +++ b/test/UndoRedoService.test.js @@ -697,6 +697,136 @@ describe('UndoRedoService', () => { await expect(service.undo()).rejects.toThrow('Cannot undo truncated batch action data'); }); + + // A single action whose file is gone can never be reversed. It used to + // stay under the cursor, so every Undo retried it and nothing older could + // be undone. + const recordMove = async (name, { landed }) => { + const originalPath = path.join(testDir, `${name}-from.txt`); + const newPath = path.join(testDir, 'sorted', `${name}.txt`); + await fs.mkdir(path.dirname(newPath), { recursive: true }); + if (landed) await fs.writeFile(newPath, name); + await service.recordAction('FILE_MOVE', { originalPath, newPath }); + return { originalPath, newPath }; + }; + + test('a single move whose file is gone is dropped, reported, and no longer blocks older entries', async () => { + const older = await recordMove('older', { landed: true }); + const gone = await recordMove('gone', { landed: false }); + const goneId = service.actions[1].id; + const changes = []; + service.onStateChanged((change) => changes.push(change.reason)); + + const result = await service.undo(); + + expect(result).toEqual( + expect.objectContaining({ + success: false, + code: 'UNDO_SOURCE_MISSING', + removedFromHistory: true, + action: expect.objectContaining({ id: goneId, type: 'FILE_MOVE' }), + pathMutations: [] + }) + ); + // The user is told which entry failed, why, and that it is gone. + expect(result.message).toContain('gone-from.txt'); + expect(result.message).toMatch(/moved or deleted outside the app/); + expect(result.message).toMatch(/removed from history/); + expect(result.error).toContain(gone.newPath); + expect(service.actions).toHaveLength(1); + expect(service.currentIndex).toBe(0); + expect(service.canRedo()).toBe(false); + expect(changes).toContain('remove'); + const persisted = JSON.parse(await fs.readFile(service.actionsPath, 'utf8')); + expect(persisted.actions.map((a) => a.id)).not.toContain(goneId); + + // The older entry is reachable again. + const second = await service.undo(); + expect(second.success).toBe(true); + await expect(fs.readFile(older.originalPath, 'utf8')).resolves.toBe('older'); + expect(service.currentIndex).toBe(-1); + }); + + test('an occupied original location keeps the entry and the error carries its code', async () => { + const move = await recordMove('blocked', { landed: true }); + await fs.writeFile(move.originalPath, 'someone else'); + + const error = await service.undo().then( + () => null, + (e) => e + ); + + expect(error).toBeInstanceOf(Error); + expect(error.code).toBe('UNDO_DESTINATION_OCCUPIED'); + expect(error.message).toMatch(/^Failed to undo action: /); + // Fixable by the user, so the entry stays for a retry. + expect(service.actions).toHaveLength(1); + expect(service.currentIndex).toBe(0); + await expect(fs.readFile(move.originalPath, 'utf8')).resolves.toBe('someone else'); + + await fs.unlink(move.originalPath); + await expect(service.undo()).resolves.toEqual(expect.objectContaining({ success: true })); + }); + + test('a missing file on a volume that does not answer keeps the entry for a retry', async () => { + const move = await recordMove('offline', { landed: false }); + const root = path.parse(move.newPath).root; + const realAccess = fs.access; + const accessSpy = jest.spyOn(fs, 'access').mockImplementation(async (target, ...rest) => { + if (String(target) === root) { + throw Object.assign(new Error('volume offline'), { code: 'ENOENT' }); + } + return realAccess.call(fs, target, ...rest); + }); + + try { + const error = await service.undo().then( + () => null, + (e) => e + ); + expect(error).toBeInstanceOf(Error); + expect(error.code).toBe('UNDO_SOURCE_MISSING'); + expect(service.actions).toHaveLength(1); + expect(service.currentIndex).toBe(0); + } finally { + accessSpy.mockRestore(); + } + }); + + test.each([ + '/Volumes/StratoSortNoSuchVolume/sorted/a.txt', + '/media/nobody/StratoSortNoSuchVolume/a.txt', + '/run/media/nobody/StratoSortNoSuchVolume/a.txt', + '/mnt/StratoSortNoSuchVolume/a.txt' + ])('a file on an ejected volume (%s) does not read as deleted', async (filePath) => { + // "/" always answers, so the mount point is what has to be probed. + await expect(service._isGoneForGood(filePath)).resolves.toBe(false); + }); + + test('a single delete whose backup is gone is dropped with its own code', async () => { + await recordMove('older', { landed: true }); + const originalPath = path.join(testDir, 'deleted.txt'); + await service.recordAction('FILE_DELETE', { + originalPath, + backupPath: path.join(testDir, 'backups', 'deleted.txt.bak'), + createBackup: true + }); + + const result = await service.undo(); + + expect(result).toEqual( + expect.objectContaining({ + success: false, + code: 'UNDO_BACKUP_MISSING', + removedFromHistory: true + }) + ); + expect(result.message).toMatch(/backup/); + expect(result.message).toMatch(/removed from history/); + expect(service.actions).toHaveLength(1); + expect(service.actions[0].type).toBe('FILE_MOVE'); + expect(service.currentIndex).toBe(0); + }); }); describe('redo', () => { @@ -722,6 +852,63 @@ describe('UndoRedoService', () => { await expect(service.redo()).rejects.toThrow('Cannot redo truncated batch action data'); }); + + test('a single move whose file is gone is dropped from the redo side and reported', async () => { + const originalPath = path.join(testDir, 'redo-from.txt'); + const newPath = path.join(testDir, 'sorted', 'redo.txt'); + const laterFrom = path.join(testDir, 'later-from.txt'); + const laterTo = path.join(testDir, 'sorted', 'later.txt'); + await fs.mkdir(path.dirname(newPath), { recursive: true }); + await fs.writeFile(newPath, 'redo'); + await fs.writeFile(laterTo, 'later'); + await service.recordAction('FILE_MOVE', { originalPath, newPath }); + await service.recordAction('FILE_MOVE', { originalPath: laterFrom, newPath: laterTo }); + await service.undo(); + await service.undo(); + expect(service.currentIndex).toBe(-1); + // The first file is deleted outside the app before the redo. + await fs.unlink(originalPath); + + const result = await service.redo(); + + expect(result).toEqual( + expect.objectContaining({ + success: false, + code: 'UNDO_SOURCE_MISSING', + removedFromHistory: true, + pathMutations: [] + }) + ); + expect(result.message).toMatch(/^Cannot redo /); + expect(result.message).toMatch(/removed from history/); + expect(service.actions).toHaveLength(1); + expect(service.currentIndex).toBe(-1); + + // The entry behind it can be redone now. + await expect(service.redo()).resolves.toEqual(expect.objectContaining({ success: true })); + await expect(fs.readFile(laterTo, 'utf8')).resolves.toBe('later'); + }); + + test('an occupied destination keeps the entry and the error carries its code', async () => { + const originalPath = path.join(testDir, 'redo-blocked-from.txt'); + const newPath = path.join(testDir, 'sorted', 'redo-blocked.txt'); + await fs.mkdir(path.dirname(newPath), { recursive: true }); + await fs.writeFile(newPath, 'mine'); + await service.recordAction('FILE_MOVE', { originalPath, newPath }); + await service.undo(); + await fs.writeFile(newPath, 'someone else'); + + const error = await service.redo().then( + () => null, + (e) => e + ); + + expect(error).toBeInstanceOf(Error); + expect(error.code).toBe('UNDO_DESTINATION_OCCUPIED'); + expect(service.actions).toHaveLength(1); + expect(service.currentIndex).toBe(-1); + expect(service.canRedo()).toBe(true); + }); }); describe('shutdown', () => { diff --git a/test/VisionService.test.js b/test/VisionService.test.js index d94dad5..9aeec56 100644 --- a/test/VisionService.test.js +++ b/test/VisionService.test.js @@ -567,6 +567,122 @@ describe('VisionService', () => { expect(service.hasInFlightRequests()).toBe(false); }); + describe('two images at once on the one-request runtime', () => { + /** + * Requests stay open until the test ends them, as on a runtime that + * is still working. `sent` lists them in the order they went out. + */ + function heldRequests() { + const http = require('http'); + const sent = []; + const spy = jest.spyOn(http, 'request').mockImplementation((options, callback) => { + const req = new EventEmitter(); + const entry = { open: false }; + req.setTimeout = jest.fn(); + req.write = jest.fn(); + req.destroy = jest.fn((error) => { + entry.open = false; + if (error) req.emit('error', error); + }); + req.end = jest.fn(() => { + entry.open = true; + }); + entry.timeOut = () => { + entry.open = false; + req.emit('error', new Error('Vision request timed out')); + }; + entry.answer = (content) => { + entry.open = false; + const res = new EventEmitter(); + res.statusCode = 200; + callback(res); + res.emit('data', JSON.stringify({ choices: [{ message: { content } }] })); + res.emit('end'); + }; + sent.push(entry); + return req; + }); + return { sent, spy }; + } + + const turn = () => new Promise((resolve) => setImmediate(resolve)); + + test('a request that times out does not restart the runtime under the other image', async () => { + await startReadyServer(); + const { sent, spy } = heldRequests(); + /** Requests on the wire each time the runtime was killed. */ + const openAtKill = []; + const runningProcess = () => { + const proc = exitingProcess(); + proc.kill = jest.fn(() => { + openAtKill.push(sent.filter((entry) => entry.open).length); + }); + return proc; + }; + service._process = runningProcess(); + // The restart: the next image finds no runtime and starts one. + service._ensureServer = jest.fn(async () => { + if (!service._process) { + service._process = runningProcess(); + service._port = 4321; + } + }); + + try { + const first = service.analyzeImage(image).catch((error) => error); + const second = service.analyzeImage(image).catch((error) => error); + await turn(); + expect(service.hasInFlightRequests()).toBe(true); + + sent[0].timeOut(); + expect((await first).message).toBe('Vision request timed out'); + + for (let i = 0; i < 5 && sent.length < 2; i += 1) await turn(); + sent[sent.length - 1].answer('a cat'); + + expect(openAtKill).toEqual([0]); + expect(await second).toEqual({ response: 'a cat' }); + expect(service.hasInFlightRequests()).toBe(false); + } finally { + spy.mockRestore(); + } + }); + + test('an image cancelled while it waits its turn leaves the runtime and the other image alone', async () => { + await startReadyServer(); + service._ensureServer = jest.fn().mockResolvedValue(undefined); + service.shutdown = jest.fn().mockResolvedValue(undefined); + const { sent, spy } = heldRequests(); + const controller = new AbortController(); + + try { + const first = service.analyzeImage(image); + const second = service + .analyzeImage({ ...image, signal: controller.signal }) + .catch((error) => error); + await turn(); + + controller.abort(); + const cancelled = await second; + expect(cancelled.name).toBe('AbortError'); + expect(sent).toHaveLength(1); + expect(service.shutdown).not.toHaveBeenCalled(); + + sent[0].answer('a dog'); + await expect(first).resolves.toEqual({ response: 'a dog' }); + + // The cancelled image gave its turn back: the next one runs. + const third = service.analyzeImage(image); + for (let i = 0; i < 5 && sent.length < 2; i += 1) await turn(); + sent[1].answer('a bird'); + await expect(third).resolves.toEqual({ response: 'a bird' }); + expect(service.hasInFlightRequests()).toBe(false); + } finally { + spy.mockRestore(); + } + }); + }); + test('a request that began before the runtime registered is held once and released once', async () => { const end = service._beginRequest(); expect(manager.acquireRef).not.toHaveBeenCalled(); diff --git a/test/analysis-fallbackUtils.test.js b/test/analysis-fallbackUtils.test.js index a4fd86a..b910762 100644 --- a/test/analysis-fallbackUtils.test.js +++ b/test/analysis-fallbackUtils.test.js @@ -101,6 +101,8 @@ describe('Analysis Fallback Utils', () => { test('returns Archives for archive files', () => { expect(getIntelligentCategory('files.zip', '.zip')).toBe('Archives'); + expect(getIntelligentCategory('files.tgz', '.tgz')).toBe('Archives'); + expect(getIntelligentCategory('files.bz2', '.bz2')).toBe('Archives'); }); test('returns Documents for unknown extensions', () => { @@ -191,6 +193,50 @@ describe('Analysis Fallback Utils', () => { const folders = [{ name: 'Financial' }, { name: 'Documents' }, { name: 'Uncategorized' }]; expect(getIntelligentCategory('invoice_2024.pdf', '.pdf', folders)).toBe('Financial'); }); + + describe('an extension listed in a folder description', () => { + test('letters inside a longer word are not the extension', () => { + // "ai" is in "email", "details" and "training"; "mov" is in "removed". + expect( + getIntelligentCategory('logo.ai', '.ai', [ + { name: 'Correspondence', description: 'Email details and training' } + ]) + ).not.toBe('Correspondence'); + expect( + getIntelligentCategory('clip.mov', '.mov', [ + { name: 'Cleanup', description: 'Removed items' } + ]) + ).not.toBe('Cleanup'); + expect( + getIntelligentCategory('run.sh', '.sh', [ + { name: 'Money', description: 'Shipping and cash' } + ]) + ).not.toBe('Money'); + }); + + test('the extension as its own word still matches, with or without the dot', () => { + expect( + getIntelligentCategory('part.xyz', '.xyz', [ + { name: 'Lab', description: 'Instrument output: .xyz, .abc' } + ]) + ).toBe('Lab'); + expect( + getIntelligentCategory('part.xyz', '.xyz', [ + { name: 'Lab', description: 'xyz files from the instrument' } + ]) + ).toBe('Lab'); + expect( + getIntelligentCategory('part.xyz', '.xyz', [ + { name: 'Lab', description: 'Everything saved as XYZ.' } + ]) + ).toBe('Lab'); + expect( + getIntelligentCategory('main.c++', '.c++', [ + { name: 'Code', description: 'Sources (.c++/.h++)' } + ]) + ).toBe('Code'); + }); + }); }); }); diff --git a/test/analysisHistory-indexManager.test.js b/test/analysisHistory-indexManager.test.js index d105711..1e89986 100644 --- a/test/analysisHistory-indexManager.test.js +++ b/test/analysisHistory-indexManager.test.js @@ -126,6 +126,24 @@ describe('indexManager', () => { expect(index.pathLookup['/path/file.pdf']).toBe('entry-1'); }); + test('indexes a moved entry under its current path only', () => { + const index = indexManager.createEmptyIndex('2.0'); + const entry = { + id: 'entry-1', + fileHash: 'hash123', + originalPath: '/downloads/file.pdf', + organization: { actual: '/docs/file.pdf' }, + timestamp: '2024-01-15T10:00:00Z', + fileSize: 1000, + analysis: { tags: [], category: null } + }; + + indexManager.updateIndexes(index, entry); + + expect(index.pathLookup['/docs/file.pdf']).toBe('entry-1'); + expect(index.pathLookup['/downloads/file.pdf']).toBeUndefined(); + }); + test('updates tag index', () => { const index = indexManager.createEmptyIndex('2.0'); const entry = { @@ -340,6 +358,26 @@ describe('indexManager', () => { expect(index.dateIndex['2024-01']).not.toContain('entry-1'); }); + test('leaves a path key that another entry owns now', () => { + const index = indexManager.createEmptyIndex('2.0'); + // entry-1 moved out of /downloads; entry-2 is a different file there. + index.pathLookup['/downloads/file.pdf'] = 'entry-2'; + index.pathLookup['/docs/file.pdf'] = 'entry-1'; + + indexManager.removeFromIndexes(index, { + id: 'entry-1', + fileHash: 'hash123', + originalPath: '/downloads/file.pdf', + organization: { actual: '/docs/file.pdf' }, + timestamp: '2024-01-15T10:00:00Z', + fileSize: 500, + analysis: { tags: [], category: null } + }); + + expect(index.pathLookup['/docs/file.pdf']).toBeUndefined(); + expect(index.pathLookup['/downloads/file.pdf']).toBe('entry-2'); + }); + test('removes from size index', () => { const index = indexManager.createEmptyIndex('2.0'); index.sizeIndex['tiny'] = ['entry-1', 'entry-2']; @@ -358,4 +396,39 @@ describe('indexManager', () => { expect(index.sizeIndex['tiny']).not.toContain('entry-1'); }); }); + + describe('updatePathIndexForMove', () => { + const entry = { id: 'entry-1', originalPath: '/downloads/file.pdf' }; + + test('a first move drops the original path and adds the new one', () => { + const index = indexManager.createEmptyIndex('2.0'); + index.pathLookup['/downloads/file.pdf'] = 'entry-1'; + + indexManager.updatePathIndexForMove(index, entry, null, '/docs/file.pdf'); + + expect(index.pathLookup).toEqual({ '/docs/file.pdf': 'entry-1' }); + }); + + test('a move back to the original path indexes it again', () => { + const index = indexManager.createEmptyIndex('2.0'); + index.pathLookup['/docs/file.pdf'] = 'entry-1'; + + indexManager.updatePathIndexForMove(index, entry, '/docs/file.pdf', '/downloads/file.pdf'); + + expect(index.pathLookup).toEqual({ '/downloads/file.pdf': 'entry-1' }); + }); + + test('keeps the key another entry holds for the path that was left', () => { + const index = indexManager.createEmptyIndex('2.0'); + index.pathLookup['/downloads/file.pdf'] = 'entry-2'; + index.pathLookup['/docs/file.pdf'] = 'entry-1'; + + indexManager.updatePathIndexForMove(index, entry, '/docs/file.pdf', '/archive/file.pdf'); + + expect(index.pathLookup).toEqual({ + '/downloads/file.pdf': 'entry-2', + '/archive/file.pdf': 'entry-1' + }); + }); + }); }); diff --git a/test/analysisHistory-queries.test.js b/test/analysisHistory-queries.test.js index 0b52159..bf0271f 100644 --- a/test/analysisHistory-queries.test.js +++ b/test/analysisHistory-queries.test.js @@ -111,6 +111,24 @@ describe('queries', () => { expect(result).toBeNull(); }); + // A moved entry keeps its originalPath; an index saved before moves + // dropped that key can still map the old path to it. Another file may + // live at the old path now, so the stale key must not resolve. + test('does not resolve a path the entry has moved away from', () => { + const moved = { + id: 'entry-1', + originalPath: '/downloads/report.pdf', + organization: { actual: '/docs/report.pdf' } + }; + const history = { entries: { 'entry-1': moved } }; + const index = { + pathLookup: { '/downloads/report.pdf': 'entry-1', '/docs/report.pdf': 'entry-1' } + }; + + expect(queries.getAnalysisByPath(history, index, '/downloads/report.pdf')).toBeNull(); + expect(queries.getAnalysisByPath(history, index, '/docs/report.pdf')).toBe(moved); + }); + // Regression: queries.normalizePathForLookup must match the write-side // canonicalization in indexManager.js (`normalizePathForIndex`). Previously // the lookup helper used a weaker normalizer that missed NFC/NFD differences diff --git a/test/analysisHistory.indexConsistency.test.js b/test/analysisHistory.indexConsistency.test.js index bcd6fbf..b9b034f 100644 --- a/test/analysisHistory.indexConsistency.test.js +++ b/test/analysisHistory.indexConsistency.test.js @@ -169,3 +169,148 @@ describe('analysis history index consistency on load', () => { expect(second.analysisIndex.pathLookup[entry.originalPath]).toBe(entry.id); }); }); + +// An entry keeps `originalPath` for good; once the file is moved, the entry +// lives at `organization.actual`. A different file that later shows up at the +// old path (a second Downloads/report.pdf) must not be mistaken for it. +describe('a moved entry is not addressed by the path it left', () => { + const services = []; + const OLD = '/downloads/report.pdf'; + const MOVED = '/docs/report.pdf'; + + beforeEach(() => { + try { + fsSync.rmSync(USER_DATA, { recursive: true, force: true }); + } catch { + // first run + } + fsSync.mkdirSync(USER_DATA, { recursive: true }); + }); + + afterEach(async () => { + for (const s of services.splice(0)) { + try { + await s.shutdown(); + } catch { + // already shut down + } + } + }); + + async function analysedThenMoved() { + const service = newService(); + services.push(service); + await service.initialize(); + await recordAndFlush(service, OLD); + const [first] = Object.values(service.analysisHistory.entries); + await service.updateEntryPaths([{ oldPath: OLD, newPath: MOVED }]); + return { service, first }; + } + + test('analysing a new file at the old path keeps the moved file in history', async () => { + const { service, first } = await analysedThenMoved(); + first.accessCount = 7; + + await recordAndFlush(service, OLD); + + const entries = Object.values(service.analysisHistory.entries); + expect(entries).toHaveLength(2); + expect(service.analysisHistory.entries[first.id]).toBe(first); + expect((await service.getAnalysisByPath(MOVED))?.id).toBe(first.id); + const second = await service.getAnalysisByPath(OLD); + expect(second?.id).not.toBe(first.id); + expect(second?.originalPath).toBe(OLD); + // Usage belongs to the moved file, not to the newcomer. + expect(second?.accessCount).toBe(0); + expect(service.analysisHistory.totalAnalyzed).toBe(2); + }); + + test('moving an unanalysed file from the old path leaves the moved entry alone', async () => { + const { service, first } = await analysedThenMoved(); + + const result = await service.updateEntryPaths([ + { + oldPath: OLD, + newPath: '/photos/renamed.pdf', + newName: 'renamed.pdf', + newCategory: 'Photos' + } + ]); + + expect(result).toMatchObject({ updated: 0, notFound: 1, divergences: [] }); + expect(first.organization.actual).toBe(MOVED); + expect(first.fileName).toBe('report.pdf'); + expect(first.analysis.category).toBe('docs'); + expect((await service.getAnalysisByPath(MOVED))?.id).toBe(first.id); + expect(await service.getAnalysisByPath('/photos/renamed.pdf')).toBeNull(); + }); + + test('moving a second analysed file from the old path moves only its own entry', async () => { + const { service, first } = await analysedThenMoved(); + await recordAndFlush(service, OLD); + const second = await service.getAnalysisByPath(OLD); + + const result = await service.updateEntryPaths([{ oldPath: OLD, newPath: '/photos/b.pdf' }]); + + expect(result).toMatchObject({ updated: 1, notFound: 0 }); + expect(first.organization.actual).toBe(MOVED); + expect(second.organization.actual).toBe('/photos/b.pdf'); + expect((await service.getAnalysisByPath(MOVED))?.id).toBe(first.id); + expect((await service.getAnalysisByPath('/photos/b.pdf'))?.id).toBe(second.id); + expect(await service.getAnalysisByPath(OLD)).toBeNull(); + }); + + test('the old path no longer resolves, and resolves again after the move is undone', async () => { + const { service, first } = await analysedThenMoved(); + + expect(await service.getAnalysisByPath(OLD)).toBeNull(); + + await service.updateEntryPaths([{ oldPath: MOVED, newPath: OLD }]); + expect((await service.getAnalysisByPath(OLD))?.id).toBe(first.id); + expect(await service.getAnalysisByPath(MOVED)).toBeNull(); + }); + + test('deleting, tagging or flagging the old path does not touch the moved entry', async () => { + const { service, first } = await analysedThenMoved(); + + expect(await service.removeEntriesByPath(OLD)).toEqual({ removed: 0 }); + expect(await service.updateTags(OLD, ['wrong-file'])).toMatchObject({ updated: 0 }); + expect(await service.updateEmbeddingStateByPath(OLD, { status: 'skipped' })).toMatchObject({ + updated: 0 + }); + + expect(service.analysisHistory.entries[first.id]).toBe(first); + expect(first.analysis.tags).not.toContain('wrong-file'); + expect(first.embedding.status).not.toBe('skipped'); + expect((await service.getAnalysisByPath(MOVED))?.id).toBe(first.id); + }); + + test('removing the moved entry keeps the newcomer at the old path findable', async () => { + const { service, first } = await analysedThenMoved(); + await recordAndFlush(service, OLD); + const second = await service.getAnalysisByPath(OLD); + + expect(await service.removeEntriesByPath(MOVED)).toEqual({ removed: 1 }); + + expect(service.analysisHistory.entries[first.id]).toBeUndefined(); + expect((await service.getAnalysisByPath(OLD))?.id).toBe(second.id); + }); + + test('an index rebuilt on load does not bring the old path back', async () => { + const path = require('path'); + const { service, first } = await analysedThenMoved(); + await service.shutdown(); + + const historyPath = path.join(USER_DATA, 'analysis-history.json'); + const history = JSON.parse(fsSync.readFileSync(historyPath, 'utf8')); + history.pairRevision = 'revision-not-in-index'; + fsSync.writeFileSync(historyPath, JSON.stringify(history)); + + const reloaded = newService(); + services.push(reloaded); + await reloaded.initialize(); + + expect(await reloaded.getAnalysisByPath(OLD)).toBeNull(); + expect((await reloaded.getAnalysisByPath(MOVED))?.id).toBe(first.id); + }); +}); diff --git a/test/atomicFileOperations.crossDeviceCollision.test.js b/test/atomicFileOperations.crossDeviceCollision.test.js index df0f107..79112fe 100644 --- a/test/atomicFileOperations.crossDeviceCollision.test.js +++ b/test/atomicFileOperations.crossDeviceCollision.test.js @@ -97,3 +97,118 @@ describe('cross-device move losing the name to another writer', () => { expect(await fsp.readFile(source, 'utf8')).toBe('MINE'); }); }); + +/** + * A caller that gives up on a cross-device move (cancel, strict-mode + * rollback) aborts its signal. The copy itself cannot be interrupted, but + * nothing may be published or deleted afterwards: the source stays, the + * destination name stays free and the staging copy is removed. + */ +describe('cross-device move aborted by its caller', () => { + /** @type {string} */ + let root; + /** @type {string} */ + let source; + /** @type {string} */ + let destination; + + beforeEach(async () => { + root = await fsp.mkdtemp(path.join(realOs.tmpdir(), 'ss-exdev-abort-')); + source = path.join(root, 'vol1', 'video.mp4'); + destination = path.join(root, 'vol2', 'video.mp4'); + await fsp.mkdir(path.dirname(source), { recursive: true }); + await fsp.mkdir(path.dirname(destination), { recursive: true }); + await fsp.writeFile(source, 'MINE'); + }); + + afterEach(async () => { + jest.restoreAllMocks(); + await fsp.rm(root, { recursive: true, force: true }); + }); + + async function expectNothingMoved() { + expect(await fsp.readFile(source, 'utf8')).toBe('MINE'); + expect(await fsp.readdir(path.dirname(destination))).toEqual([]); + } + + test('abort during the copy: nothing is published and the source stays', async () => { + const controller = new AbortController(); + const realCopy = fsp.copyFile.bind(fsp); + jest.spyOn(fsp, 'copyFile').mockImplementation(async (from, to, mode) => { + await realCopy(from, to, mode); + controller.abort('user-cancel-organize'); + }); + const checksumFn = jest.fn(async () => 'same'); + + const error = await crossDeviceMove(source, destination, { + checksumFn, + signal: controller.signal + }).then( + () => null, + (e) => e + ); + + expect(error).toMatchObject({ name: 'AbortError' }); + // The verification pass (two full-file hashes) is skipped too. + expect(checksumFn).not.toHaveBeenCalled(); + await expectNothingMoved(); + }); + + test('abort during verification: nothing is published and the source stays', async () => { + const controller = new AbortController(); + const checksumFn = jest.fn(async () => { + controller.abort(); + return 'same'; + }); + + const error = await crossDeviceMove(source, destination, { + checksumFn, + signal: controller.signal + }).then( + () => null, + (e) => e + ); + + expect(error).toMatchObject({ name: 'AbortError' }); + await expectNothingMoved(); + }); + + test('abort landing with the publish: the copy is withdrawn and the source stays', async () => { + const controller = new AbortController(); + const realRename = fsp.rename.bind(fsp); + jest.spyOn(fsp, 'rename').mockImplementation(async (from, to) => { + await realRename(from, to); + controller.abort(); + }); + + const error = await crossDeviceMove(source, destination, { signal: controller.signal }).then( + () => null, + (e) => e + ); + + expect(error).toMatchObject({ name: 'AbortError' }); + await expectNothingMoved(); + }); + + test('a signal that never aborts changes nothing', async () => { + await crossDeviceMove(source, destination, { signal: new AbortController().signal }); + + expect(await fsp.readFile(destination, 'utf8')).toBe('MINE'); + await expect(fsp.access(source)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + test('a signal already aborted copies nothing', async () => { + const controller = new AbortController(); + controller.abort(); + const copySpy = jest.spyOn(fsp, 'copyFile'); + + const error = await crossDeviceMove(source, destination, { signal: controller.signal }).then( + () => null, + (e) => e + ); + + expect(error).toMatchObject({ name: 'AbortError' }); + expect(copySpy).not.toHaveBeenCalled(); + await expectNothingMoved(); + }); +}); diff --git a/test/autoUpdater.test.js b/test/autoUpdater.test.js index 0329ce4..f4499d7 100644 --- a/test/autoUpdater.test.js +++ b/test/autoUpdater.test.js @@ -533,6 +533,65 @@ describe('autoUpdater', () => { // list that status, and strict safeSend blocked it with an error log on // every launch. Drive every path that sends APP_UPDATE and validate each // payload against the real schema so sender and contract cannot drift. + // electron-updater keeps a downloaded installer and still installs it on + // quit when a later check fails, so the app must keep saying so. + describe('a failed check after a completed download', () => { + function sentStatuses() { + const { safeSend } = require('../src/main/ipc/ipcWrappers'); + return safeSend.mock.calls.map((call) => call[2]?.status); + } + + async function initWithDownloadedUpdate() { + process.env.STRATOSORT_ENABLE_AUTO_UPDATES = 'true'; + const mockWindow = { isDestroyed: jest.fn().mockReturnValue(false), webContents: {} }; + mockGetAllWindows.mockReturnValue([mockWindow]); + + const updater = require('../src/main/core/autoUpdater'); + await updater.initializeAutoUpdater(false, jest.fn().mockReturnValue(mockWindow)); + mockAutoUpdaterHandlers.get('update-downloaded')(); + expect(updater.hasPendingDownloadedUpdate()).toBe(true); + return updater; + } + + testIfSupported('an updater error keeps the downloaded update pending and shown', async () => { + const updater = await initWithDownloadedUpdate(); + + mockAutoUpdaterHandlers.get('error')(new Error('feed returned 503')); + + expect(updater.hasPendingDownloadedUpdate()).toBe(true); + // The renderer hides "Update ready" on an error status. + const statuses = sentStatuses(); + expect(statuses.slice(statuses.lastIndexOf('downloaded'))).toEqual(['downloaded']); + // The failure is still reported on the degraded-state banner. + const flags = require('../src/main/services/DegradedStateTracker') + .getInstance() + .list() + .filter((flag) => flag.code === 'auto-updater.unavailable'); + expect(flags).toHaveLength(1); + }); + + testIfSupported('a manual check that fails keeps the downloaded update pending', async () => { + const updater = await initWithDownloadedUpdate(); + mockAutoUpdater.checkForUpdates.mockRejectedValueOnce(new Error('feed unreachable')); + + const result = await updater.checkForUpdates(); + + expect(result).toEqual({ success: false, error: 'feed unreachable' }); + expect(updater.hasPendingDownloadedUpdate()).toBe(true); + }); + + testIfSupported('a newer version found by a later check replaces the download', async () => { + const updater = await initWithDownloadedUpdate(); + + mockAutoUpdaterHandlers.get('update-available')(); + expect(updater.hasPendingDownloadedUpdate()).toBe(false); + + // With no finished download an error is reported to the renderer again. + mockAutoUpdaterHandlers.get('error')(new Error('download failed')); + expect(sentStatuses().at(-1)).toBe('error'); + }); + }); + describe('every app:update payload satisfies the event schema', () => { const setPlatform = (value) => Object.defineProperty(process, 'platform', { value, configurable: true }); @@ -587,9 +646,11 @@ describe('autoUpdater', () => { mockAutoUpdaterHandlers.get('update-available')(); mockAutoUpdaterHandlers.get('update-not-available')(); mockAutoUpdaterHandlers.get('download-progress')({ percent: 42.5 }); - mockAutoUpdaterHandlers.get('update-downloaded')(); + // Errors come before the download completes: once an update is + // downloaded, an error no longer sends a status. mockAutoUpdaterHandlers.get('error')(new Error('feed unreachable')); mockAutoUpdaterHandlers.get('error')(undefined); + mockAutoUpdaterHandlers.get('update-downloaded')(); const payloads = sentUpdatePayloads(); expect(payloads.map((p) => p.status)).toEqual( diff --git a/test/batchOrganizeHandler.test.js b/test/batchOrganizeHandler.test.js index a55e8a1..2737f2d 100644 --- a/test/batchOrganizeHandler.test.js +++ b/test/batchOrganizeHandler.test.js @@ -107,10 +107,13 @@ jest.mock('../src/shared/pathSanitization', () => ({ })); jest.mock('../src/shared/promiseUtils', () => { - // Minimal Semaphore for tests (no timeout/queue-limit overhead) + // Minimal Semaphore for tests. Like the real one, a waiter given a queue + // timeout is rejected when it runs out, so a batch that queued its moves + // behind one again would fail the long-batch test below. class MockSemaphore { - constructor(max) { + constructor(max, _maxQueue, queueTimeoutMs) { this.max = max; + this.queueTimeoutMs = queueTimeoutMs; this.active = 0; this.queue = []; } @@ -119,11 +122,22 @@ jest.mock('../src/shared/promiseUtils', () => { this.active++; return; } - await new Promise((resolve) => this.queue.push(resolve)); + await new Promise((resolve, reject) => { + const waiter = { resolve, timeout: undefined }; + if (this.queueTimeoutMs > 0) { + waiter.timeout = setTimeout(() => { + this.queue.splice(this.queue.indexOf(waiter), 1); + reject(new Error('Request queue timeout')); + }, this.queueTimeoutMs); + } + this.queue.push(waiter); + }); } release() { if (this.queue.length > 0) { - this.queue.shift()(); + const waiter = this.queue.shift(); + clearTimeout(waiter.timeout); + waiter.resolve(); } else if (this.active > 0) { this.active--; } @@ -1413,6 +1427,306 @@ describe('Batch Organize Handler', () => { expect(batchLockManager.renewBatchLock).toHaveBeenCalledWith(result.batchId); }); + // A cross-volume move copies and checksums the whole file, which takes + // longer than any fixed limit for a large one. The handler used to race the + // move against a 30 s timer that cancelled nothing: the row came back + // failed while the copy went on to publish and delete the source. + describe('a move that outlasts 30 s', () => { + const { crossDeviceMove } = require('../src/shared/atomicFileOperations'); + const promiseUtils = require('../src/shared/promiseUtils'); + const flush = () => new Promise((resolve) => setImmediate(resolve)); + const exdev = () => Object.assign(new Error('EXDEV: cross-device link'), { code: 'EXDEV' }); + const abortError = () => Object.assign(new Error('Move aborted'), { name: 'AbortError' }); + + /** A crossDeviceMove that copies until `finishCopy()` and honors its signal like the real one. */ + function stubSlowCrossDeviceMove() { + /** @type {string[]} */ + const moved = []; + /** @type {() => void} */ + let finishCopy = () => {}; + const copying = new Promise((resolve) => { + finishCopy = () => resolve(undefined); + }); + crossDeviceMove.mockImplementation(async (source, _dest, options) => { + await copying; + if (options?.signal?.aborted) throw abortError(); + moved.push(source); + }); + return { moved, finishCopy }; + } + + async function waitForCopyToStart() { + for (let i = 0; i < 50 && crossDeviceMove.mock.calls.length === 0; i++) await flush(); + expect(crossDeviceMove).toHaveBeenCalledTimes(1); + } + + beforeEach(() => { + fs.rename.mockImplementation(async (source) => { + if (source === '/src/doc1.pdf') throw exdev(); + }); + }); + + afterEach(() => { + jest.useRealTimers(); + fs.rename.mockReset().mockResolvedValue(); + crossDeviceMove.mockReset(); + promiseUtils.withTimeout.mockReset().mockImplementation((promise) => promise); + }); + + test('is reported as moved, with its undo entry, and keeps the batch lock renewed meanwhile', async () => { + jest.useFakeTimers({ doNotFake: ['setImmediate', 'nextTick', 'queueMicrotask'] }); + // The real race, so the 30 s limit is in force wherever the handler still uses it. + promiseUtils.withTimeout.mockImplementation( + jest.requireActual('../src/shared/promiseUtils').withTimeout + ); + const svc = makeStableSvc(); + const { moved, finishCopy } = stubSlowCrossDeviceMove(); + + const pending = handleBatchOrganize({ + ...params, + getServiceIntegration: jest.fn(() => svc), + operation: { + operations: [{ source: '/src/doc1.pdf', destination: '/dest/doc1.pdf', type: 'move' }] + } + }); + await waitForCopyToStart(); + + await jest.advanceTimersByTimeAsync(95 * 1000); + await flush(); + const renewalsDuringCopy = batchLockManager.renewBatchLock.mock.calls.length; + finishCopy(); + const result = await pending; + + expect(moved).toEqual(['/src/doc1.pdf']); + expect(result.results).toEqual([ + expect.objectContaining({ + source: '/src/doc1.pdf', + destination: '/dest/doc1.pdf', + success: true + }) + ]); + expect(result.failCount).toBe(0); + expect(result.completedOperations).toBe(1); + expect(svc.undoRedo.recordAction).toHaveBeenCalledTimes(1); + expect(svc.undoRedo.recordAction.mock.calls[0][1].operations).toEqual([ + { type: 'move', originalPath: '/src/doc1.pdf', newPath: '/dest/doc1.pdf' } + ]); + expect(svc.processingState.markOrganizeOpError).not.toHaveBeenCalled(); + expect(renewalsDuringCopy).toBeGreaterThanOrEqual(3); + expect(batchLockManager.renewBatchLock).toHaveBeenCalledWith(result.batchId); + // The renewal stops with the move. + expect(jest.getTimerCount()).toBe(0); + }); + + // The batch had three 5-minute limits of its own (a wait-for-a-slot + // timeout, an elapsed-time check, the renderer's request timeout). A + // batch of large files going to another drive hit them while every move + // was healthy: the files already moved were moved back. + test('a batch still copying after 5 minutes finishes; a file waiting its turn is moved, nothing is rolled back', async () => { + jest.useFakeTimers({ doNotFake: ['setImmediate', 'nextTick', 'queueMicrotask'] }); + const svc = makeStableSvc(); + // Five large files fill every slot; the sixth waits for one. + const bigFiles = [1, 2, 3, 4, 5].map((n) => `/src/big${n}.mp4`); + fs.rename.mockImplementation(async (source) => { + if (bigFiles.includes(source)) throw exdev(); + }); + const { moved, finishCopy } = stubSlowCrossDeviceMove(); + + const pending = handleBatchOrganize({ + ...params, + getServiceIntegration: jest.fn(() => svc), + operation: { + operations: [ + ...bigFiles.map((source) => ({ + source, + destination: source.replace('/src/', '/dest/'), + type: 'move' + })), + { source: '/src/doc6.pdf', destination: '/dest/doc6.pdf', type: 'move' } + ] + } + }); + for (let i = 0; i < 50 && crossDeviceMove.mock.calls.length < 5; i++) await flush(); + expect(crossDeviceMove).toHaveBeenCalledTimes(5); + + await jest.advanceTimersByTimeAsync(6 * 60 * 1000); + await flush(); + finishCopy(); + const result = await pending; + + const { executeRollback } = require('../src/main/ipc/files/batchRollback'); + expect(executeRollback).not.toHaveBeenCalled(); + expect(moved.sort()).toEqual(bigFiles); + expect(result.success).toBe(true); + expect(result.successCount).toBe(6); + expect(result.failCount).toBe(0); + expect(result.results).toHaveLength(6); + expect(result.results.every((row) => row.success === true)).toBe(true); + expect(svc.processingState.markOrganizeOpError).not.toHaveBeenCalled(); + expect(svc.processingState.completeOrganizeBatch).toHaveBeenCalledWith(result.batchId); + expect(jest.getTimerCount()).toBe(0); + }); + + test('strict mode: a move still copying when another file fails is stopped, not finished after the rollback', async () => { + fs.rename.mockImplementation(async (source) => { + if (source === '/src/doc1.pdf') throw exdev(); + throw Object.assign(new Error('EINVAL: invalid argument'), { code: 'EINVAL' }); + }); + const { moved, finishCopy } = stubSlowCrossDeviceMove(); + + const pending = handleBatchOrganize({ + ...params, + operation: { ...params.operation, strict: true } + }); + await waitForCopyToStart(); + for (let i = 0; i < 20; i++) await flush(); + finishCopy(); + const result = await pending; + + expect(moved).toEqual([]); + expect(crossDeviceMove.mock.calls[0][2].signal.aborted).toBe(true); + expect(result.success).toBe(false); + expect(result.results.filter((row) => row.success)).toEqual([]); + expect(result.results).toEqual( + expect.arrayContaining([ + expect.objectContaining({ source: '/src/doc2.pdf', strictModeTriggered: true }) + ]) + ); + // Nothing moved, so there is nothing to roll back or undo. + const { executeRollback } = require('../src/main/ipc/files/batchRollback'); + expect(executeRollback).not.toHaveBeenCalled(); + }); + + test('user cancel during the copy leaves the file where it was and reports it cancelled', async () => { + const controller = new AbortController(); + const svc = makeStableSvc(); + const { moved, finishCopy } = stubSlowCrossDeviceMove(); + + const pending = handleBatchOrganize({ + ...params, + abortSignal: controller.signal, + getServiceIntegration: jest.fn(() => svc), + operation: { + operations: [{ source: '/src/doc1.pdf', destination: '/dest/doc1.pdf', type: 'move' }] + } + }); + await waitForCopyToStart(); + controller.abort('user-cancel-organize'); + finishCopy(); + const result = await pending; + + expect(moved).toEqual([]); + expect(result.cancelled).toBe(true); + expect(result.cancelledCount).toBe(1); + expect(result.failCount).toBe(0); + expect(result.results).toEqual([ + expect.objectContaining({ source: '/src/doc1.pdf', success: false, cancelled: true }) + ]); + expect(svc.processingState.markOrganizeOpError).not.toHaveBeenCalled(); + }); + + describe('on a drive that stopped answering', () => { + /** A rename of doc1 that stays open until `answer()` settles it. */ + function stubHungRename() { + /** @type {(error?: any) => void} */ + let answer = () => {}; + const hung = new Promise((resolve, reject) => { + answer = (error) => (error ? reject(error) : resolve(undefined)); + }); + fs.rename.mockImplementation(async (source) => { + if (source === '/src/doc1.pdf') await hung; + }); + return { answer: (error) => answer(error) }; + } + + async function stopAStuckBatch(reason) { + jest.useFakeTimers({ doNotFake: ['setImmediate', 'nextTick', 'queueMicrotask'] }); + const controller = new AbortController(); + const { answer } = stubHungRename(); + const pending = handleBatchOrganize({ + ...params, + abortSignal: controller.signal, + getServiceIntegration: jest.fn(() => makeStableSvc()), + operation: { + operations: [{ source: '/src/doc1.pdf', destination: '/dest/doc1.pdf', type: 'move' }] + } + }); + for (let i = 0; i < 50 && fs.rename.mock.calls.length === 0; i++) await flush(); + expect(fs.rename).toHaveBeenCalledTimes(1); + controller.abort(reason); + await jest.advanceTimersByTimeAsync(16 * 1000); + const result = await pending; + return { result, answer }; + } + + const putBackCalls = () => fs.rename.mock.calls.filter(([from]) => from === '/dest/doc1.pdf'); + + test('Stop ends the batch after a short wait, and a move that goes through later is put back', async () => { + const { result, answer } = await stopAStuckBatch('user-cancel-organize'); + + expect(result.cancelled).toBe(true); + expect(result.results).toEqual([ + expect.objectContaining({ source: '/src/doc1.pdf', success: false, cancelled: true }) + ]); + expect(batchLockManager.releaseBatchLock).toHaveBeenCalled(); + expect(jest.getTimerCount()).toBe(0); + + // The drive answers after all and the rename goes through. + answer(); + for (let i = 0; i < 20; i++) await flush(); + expect(putBackCalls()).toEqual([['/dest/doc1.pdf', '/src/doc1.pdf']]); + }); + + test('a move that fails later is left alone', async () => { + const { result, answer } = await stopAStuckBatch('user-cancel-organize'); + expect(result.cancelled).toBe(true); + + answer(Object.assign(new Error('ENETUNREACH'), { code: 'ENETUNREACH' })); + for (let i = 0; i < 20; i++) await flush(); + expect(putBackCalls()).toEqual([]); + }); + + test('an interrupted batch does not put the file back; its journal resumes it', async () => { + const { result, answer } = await stopAStuckBatch('ipc-timeout'); + expect(result.results).toEqual([ + expect.objectContaining({ source: '/src/doc1.pdf', success: false, interrupted: true }) + ]); + + answer(); + for (let i = 0; i < 20; i++) await flush(); + expect(putBackCalls()).toEqual([]); + }); + + test('a move that answers within the wait is still reported as moved', async () => { + jest.useFakeTimers({ doNotFake: ['setImmediate', 'nextTick', 'queueMicrotask'] }); + const controller = new AbortController(); + const { answer } = stubHungRename(); + const pending = handleBatchOrganize({ + ...params, + abortSignal: controller.signal, + getServiceIntegration: jest.fn(() => makeStableSvc()), + operation: { + operations: [{ source: '/src/doc1.pdf', destination: '/dest/doc1.pdf', type: 'move' }] + } + }); + for (let i = 0; i < 50 && fs.rename.mock.calls.length === 0; i++) await flush(); + controller.abort('user-cancel-organize'); + await jest.advanceTimersByTimeAsync(5 * 1000); + answer(); + const result = await pending; + + expect(result.results).toEqual([ + expect.objectContaining({ + source: '/src/doc1.pdf', + destination: '/dest/doc1.pdf', + success: true + }) + ]); + expect(putBackCalls()).toEqual([]); + }); + }); + }); + // Overlap detection folds case wherever the default filesystem does. On // darwin (APFS/HFS+ are case-insensitive by default) two destinations that // differ only by case are the SAME file: run in parallel, both diff --git a/test/components/App.viewMode.test.js b/test/components/App.viewMode.test.js index a66d5b7..c43662b 100644 --- a/test/components/App.viewMode.test.js +++ b/test/components/App.viewMode.test.js @@ -21,6 +21,11 @@ jest.mock('../../src/renderer/store/slices/uiSlice', () => ({ updateSettings: jest.fn((payload) => ({ type: 'ui/updateSettings', payload })) })); +const mockAddNotification = jest.fn(); +jest.mock('../../src/renderer/contexts/NotificationContext', () => ({ + useNotification: () => ({ addNotification: mockAddNotification }) +})); + jest.mock('../../src/shared/constants', () => ({ PHASES: { WELCOME: 'welcome', @@ -191,6 +196,65 @@ describe('App view-mode routing', () => { }); }); +describe('App expired-session notice', () => { + // store/index.js keeps viewMode across a TTL expiry, so a dashboard user + // never mounts WelcomePhase at startup. The notice has to come from the + // shell, which is mounted on both surfaces. + beforeEach(() => { + mockDispatch.mockClear(); + mockAddNotification.mockClear(); + delete window.__STRATOSORT_STATE_EXPIRED__; + delete window.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__; + }); + + afterEach(() => { + delete window.__STRATOSORT_STATE_EXPIRED__; + delete window.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__; + }); + + const expiredNotices = () => + mockAddNotification.mock.calls.filter(([message]) => + String(message).includes('Previous session') + ); + + test('is shown once at startup on the dashboard surface and clears the flag', async () => { + window.__STRATOSORT_STATE_EXPIRED__ = true; + window.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__ = 48; + setupState({ viewMode: 'dashboard', currentPhase: 'welcome', smartFolders: [{ id: 'sf-1' }] }); + const { rerender } = render(); + + expect(await screen.findByTestId('dashboard-view')).toBeInTheDocument(); + expect(screen.queryByTestId('phase-renderer')).not.toBeInTheDocument(); + expect(expiredNotices()).toHaveLength(1); + expect(expiredNotices()[0][0]).toContain('48h old'); + expect(expiredNotices()[0][1]).toBe('info'); + expect(window.__STRATOSORT_STATE_EXPIRED__).toBeUndefined(); + expect(window.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__).toBeUndefined(); + + rerender(); + expect(expiredNotices()).toHaveLength(1); + }); + + test('is shown once on the phased surface', async () => { + window.__STRATOSORT_STATE_EXPIRED__ = true; + window.__STRATOSORT_STATE_EXPIRED_AGE_HOURS__ = 30; + setupState({ viewMode: 'phased', currentPhase: 'welcome' }); + render(); + + expect(await screen.findByTestId('phase-renderer')).toBeInTheDocument(); + expect(expiredNotices()).toHaveLength(1); + expect(window.__STRATOSORT_STATE_EXPIRED__).toBeUndefined(); + }); + + test('is not shown when no session expired', async () => { + setupState({ viewMode: 'dashboard', smartFolders: [{ id: 'sf-1' }] }); + render(); + + expect(await screen.findByTestId('dashboard-view')).toBeInTheDocument(); + expect(expiredNotices()).toHaveLength(0); + }); +}); + describe('App settings overlay hoisting', () => { // The settings overlay must be mounted at App level, NOT inside // PhaseRenderer: the dashboard surface replaces PhaseRenderer entirely, so diff --git a/test/components/ChatPanel.test.js b/test/components/ChatPanel.test.js index 483cf2b..0c985c5 100644 --- a/test/components/ChatPanel.test.js +++ b/test/components/ChatPanel.test.js @@ -179,6 +179,57 @@ describe('ChatPanel', () => { ).toBeInTheDocument(); }); + test('a reopened conversation shows the same answer as the live one, not the stored memory text', () => { + // A saved assistant row keeps `text` as the prompt-memory string (document + // and model items joined, plus a "[Referenced: ...]" line) next to the + // structured answer. The bubble must render the structured answer. + const { container } = renderWithRedux( +