From 379f3def86dfa00c77fec73cb91c5f9dc6bd96d9 Mon Sep 17 00:00:00 2001 From: chriscrosstalk <49691103+chriscrosstalk@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:45:35 -0700 Subject: [PATCH] fix(kb): respect ingest policy when a ZIM is uploaded locally (#1184) ZimService.registerLocalUpload() dispatched EmbedFileJob unconditionally, gated only on whether Ollama was reachable. It never read rag.defaultIngestPolicy, so a user who deliberately chose Manual still got sideloaded ZIMs embedded into the knowledge base behind their back. PR #919 fixed exactly this for the post-download dispatch path. The local-upload path was missed. Rather than inline a third copy of the Always/Manual conditional, this reuses decideScanAction, the same helper the scanner uses. That also means an existing browse_only or pending_decision row is now honored instead of being overridden by the act of re-uploading the file, which the inline version in run_download_job.ts does not do. Unset policy is still treated as Always, so existing installs keep their current behavior. Reported by @just-jbc on #1119, which also proposed disabling ZIM auto-discovery entirely. That larger change is not included here: turning discovery off by default would mean a user who downloads Wikipedia through the curated flow gets no AI answers from it and no explanation why. Co-authored-by: Claude Opus 5 (1M context) --- admin/app/services/zim_service.ts | 38 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/admin/app/services/zim_service.ts b/admin/app/services/zim_service.ts index 2de6a36..9a5d97c 100644 --- a/admin/app/services/zim_service.ts +++ b/admin/app/services/zim_service.ts @@ -600,14 +600,40 @@ export class ZimService { const ollamaUrl = await this.dockerService.getServiceURL('nomad_ollama') if (ollamaUrl) { + // Respect the global ingest policy, same as the post-download path (PR #919). + // This used to dispatch unconditionally, so a user who deliberately chose + // Manual still got sideloaded ZIMs embedded behind their back. + // + // Reuses decideScanAction rather than re-inlining the Always/Manual check, + // so an existing browse_only or pending_decision row is honored too instead + // of being overridden by the act of re-uploading the file. + const filePath = join(process.cwd(), ZIM_STORAGE_PATH, filename) try { - const { EmbedFileJob } = await import('#jobs/embed_file_job') - await EmbedFileJob.dispatch({ - fileName: filename, - filePath: join(process.cwd(), ZIM_STORAGE_PATH, filename), - }) + const { default: KVStore } = await import('#models/kv_store') + const { default: KbIngestState } = await import('#models/kb_ingest_state') + const { decideScanAction } = await import('../utils/kb_ingest_decision.js') + + // Unset is treated as Always, preserving legacy behavior — mirrors + // rag_service.ts and run_download_job.ts. + const policyRaw = await KVStore.getValue('rag.defaultIngestPolicy') + const policy = policyRaw === 'Manual' ? 'Manual' : 'Always' + + const existing = await KbIngestState.findBy('file_path', filePath) + const action = decideScanAction(existing, false, policy) + + if (action.kind === 'dispatch') { + const { EmbedFileJob } = await import('#jobs/embed_file_job') + await EmbedFileJob.dispatch({ fileName: filename, filePath }) + } else if (action.kind === 'create_pending') { + // firstOrCreate so the KB panel surfaces the per-file Index affordance + // without demoting a row that already exists. + await KbIngestState.getOrCreate(filePath) + } + // 'skip' and 'backfill_indexed' need no action here: the file was just + // written to disk, so there is nothing to backfill and a settled state + // row means the user has already decided about this file. } catch (error) { - logger.error(`[ZimService] EmbedFileJob dispatch failed after local upload:`, error) + logger.error(`[ZimService] KB ingest decision failed after local upload:`, error) } }