From b4375e558dfc5ccf34ee6736b886e90f662b1688 Mon Sep 17 00:00:00 2001 From: SnapOtter Date: Fri, 3 Jul 2026 13:47:15 +0800 Subject: [PATCH] fix: harden install queue/dispatcher lifecycle and repair review-sweep regressions (#395) Fixes 15 defects found by a max-effort multi-agent review of the last 6 merged PRs (#388, #390, #391, #392, #393, #394), all adversarially verified before fixing. Install queue + dispatcher (the serious cluster): - features.ts: finalize the installer child exactly once. A failed spawn fires both "error" and "close", and the second event released the file lock and active slot that pump() had just handed to the next queued bundle, letting two pip processes write the same venv concurrently. Outcome recording now happens before pump() so the next bundle's first progress frame cannot race the previous install's bookkeeping. - feature-status.ts: keep failed-install errors in a per-bundle map instead of the single progress slot. With the queue auto-starting the next install, the slot was overwritten within seconds and a failed install vanished without ever surfacing to GET /features. - bridge.ts: scope child lifecycle per process (stopped-children set + request generation tags) instead of an instance-wide shuttingDown flag that the next spawn reset. A stale SIGTERMed child's late close event could record a phantom crash (5 of which permanently disable the dispatcher), null out the freshly spawned child, and reject the new child's pending requests. The request-timeout kill path still counts as a real crash. - install_feature.py: the pre-write disk re-check measured ai_dir's filesystem even when budgeting the cross-filesystem copy that lands on the venv's disk; now each budget is checked against the filesystem the bytes actually land on, so ENOSPC cannot strike mid-write and leave site-packages half overwritten. Behavior regressions: - embed-subtitles: preserve pre-existing subtitle tracks (0:s?) and MKV attachments (0:t?) that the -map 0:v:0/0:a? rewrite silently dropped; data streams stay unmapped on purpose (the actual MPEG remux fix). The new subtitle maps first so the language tag hits the right stream. - usage-survey-overlay: fail closed when the settings fetch fails; the fail-open path rendered the blocking survey against an unhealthy API and soft-locked admins, the lock-out class #392 fixed. - features-store: queued bundles poll instead of each holding an SSE connection (Install All could pin 7 EventSources and exhaust the browser's 6-per-origin HTTP/1.1 limit, hanging the whole app); listenToProgress closes any prior stream and stops any poll before subscribing; installAll skips bundles already installing or queued. Contracts, tests, i18n: - openapi.yaml: add "queued" to the features status enum and document downloadBytes/installedBytes (Schemathesis conformance). - feature-lifecycle e2e: queue transcription (~0.5 GB) instead of ocr (~6 GB) and give the test a budget that covers both install drains (the stacked waits exceeded the old 900s timeout). - docker-compose.qa.yml: parameterize the host port (QA_APP_PORT) so QA_PROJECT_NAME concurrent stacks can actually bind. - compare + watermark-image: restore per-input error attribution ("Invalid first/second image", "Invalid watermark image") lost in the shared-handler migration. - ai-features-section: the "{size} on disk" suffix now goes through i18n; key added to all 21 locales. - watermark-image + content-aware-resize: migrate to the shared inputHandlerFor("image") chain like compare/vectorize/compose, fixing drift in the inline copies (no SVG sanitize, no RAW extension hint, no AVIF probe). Verified: typecheck across 9 workspaces, Biome clean on all changed files, 584 targeted unit tests and 249 integration tests green (including real-ffmpeg embed-subtitles runs). One unit test updated to the new poll-while-queued contract with a single-EventSource assertion. Claude-Session: https://claude.ai/code/session_017mR1HiHaf3a1BmUtrHX4j3 --- apps/api/src/lib/feature-status.ts | 37 ++++-- apps/api/src/openapi.yaml | 13 +- apps/api/src/plugins/auth.ts | 2 +- apps/api/src/routes/features.ts | 38 ++++-- apps/api/src/routes/tools/compare.ts | 29 +++-- .../src/routes/tools/content-aware-resize.ts | 53 +++----- apps/api/src/routes/tools/embed-subtitles.ts | 11 ++ apps/api/src/routes/tools/watermark-image.ts | 114 +++++------------- .../onboarding/usage-survey-overlay.tsx | 8 +- .../settings/ai-features-section.tsx | 7 +- apps/web/src/stores/features-store.ts | 38 ++++-- packages/ai/python/install_feature.py | 29 ++--- packages/ai/src/bridge.ts | 113 ++++++++++------- .../src/operations/auto-enhance.ts | 6 +- packages/shared/src/i18n/ar.ts | 1 + packages/shared/src/i18n/de.ts | 1 + packages/shared/src/i18n/en.ts | 1 + packages/shared/src/i18n/es.ts | 1 + packages/shared/src/i18n/fr.ts | 1 + packages/shared/src/i18n/hi.ts | 1 + packages/shared/src/i18n/id.ts | 1 + packages/shared/src/i18n/it.ts | 1 + packages/shared/src/i18n/ja.ts | 1 + packages/shared/src/i18n/ko.ts | 1 + packages/shared/src/i18n/nl.ts | 1 + packages/shared/src/i18n/pl.ts | 1 + packages/shared/src/i18n/pt-BR.ts | 1 + packages/shared/src/i18n/ru.ts | 1 + packages/shared/src/i18n/sv.ts | 1 + packages/shared/src/i18n/th.ts | 1 + packages/shared/src/i18n/tr.ts | 1 + packages/shared/src/i18n/uk.ts | 1 + packages/shared/src/i18n/vi.ts | 1 + packages/shared/src/i18n/zh-CN.ts | 1 + packages/shared/src/i18n/zh-TW.ts | 1 + tests/e2e-docker/feature-lifecycle.spec.ts | 27 +++-- .../platform/auth-edge-cases.test.ts | 2 +- .../platform/feature-install-queue.test.ts | 2 +- .../tools/data/extract-zip.test.ts | 4 +- tests/qa/api-sweep.mts | 10 +- tests/qa/docker-compose.qa.yml | 8 +- tests/qa/install-ai-bundles-ui.mts | 6 +- tests/unit/ai/bridge.test.ts | 2 +- tests/unit/image-engine/auto-enhance.test.ts | 6 +- .../unit/web/features-store-expanded.test.ts | 2 +- tests/unit/web/features-store.test.ts | 50 +++++--- 46 files changed, 383 insertions(+), 255 deletions(-) diff --git a/apps/api/src/lib/feature-status.ts b/apps/api/src/lib/feature-status.ts index 1d7fefe0..e10bd837 100644 --- a/apps/api/src/lib/feature-status.ts +++ b/apps/api/src/lib/feature-status.ts @@ -257,9 +257,18 @@ export function getInstallingBundle(): { let currentProgress: { bundleId: string; progress: { percent: number; stage: string } | null; - error: string | null; } | null = null; +/** + * Failed-install errors keyed by bundle. Errors live outside the single + * progress slot because the queue pump starts the next install immediately + * after a failure: if the error sat in the slot, the next bundle's first + * progress frame would overwrite it and the failure would never surface to + * GET /features. An entry clears when a new install of the same bundle + * starts (its first setInstallProgress call with a null error). + */ +const installErrors = new Map(); + export function setInstallProgress( bundleId: string | null, progress: { percent: number; stage: string } | null, @@ -267,9 +276,20 @@ export function setInstallProgress( ): void { if (!bundleId) { currentProgress = null; + installErrors.clear(); return; } - currentProgress = { bundleId, progress, error }; + if (error !== null) { + installErrors.set(bundleId, error); + if (currentProgress?.bundleId === bundleId) currentProgress = null; + return; + } + installErrors.delete(bundleId); + if (progress === null) { + if (currentProgress?.bundleId === bundleId) currentProgress = null; + return; + } + currentProgress = { bundleId, progress }; } // ── Manifest reading ──────────────────────────────────────────────────── @@ -519,14 +539,15 @@ export function getFeatureStates(): FeatureBundleState[] { let error: string | null = null; let progress: { percent: number; stage: string } | null = null; + const installError = installErrors.get(bundle.id) ?? null; if (lock && lock.bundleId === bundle.id) { status = "installing"; if (currentProgress && currentProgress.bundleId === bundle.id) { progress = currentProgress.progress; - if (currentProgress.error) { - status = "error"; - error = currentProgress.error; - } + } + if (installError) { + status = "error"; + error = installError; } } else if (installedBundle) { // Verify model files exist and are properly sized @@ -540,9 +561,9 @@ export function getFeatureStates(): FeatureBundleState[] { } else if (queuedIds.has(bundle.id)) { // Waiting behind the active install in the server-side queue. status = "queued"; - } else if (currentProgress?.bundleId === bundle.id && currentProgress.error) { + } else if (installError) { status = "error"; - error = currentProgress.error; + error = installError; } const archive = manifest?.bundles[bundle.id]?.archives?.[arch]; diff --git a/apps/api/src/openapi.yaml b/apps/api/src/openapi.yaml index 357c2612..c5e531ad 100644 --- a/apps/api/src/openapi.yaml +++ b/apps/api/src/openapi.yaml @@ -6450,11 +6450,22 @@ paths: description: Human-readable description of the feature bundle status: type: string - enum: [installed, not_installed, installing, error] + enum: [installed, not_installed, installing, queued, error] installedVersion: type: [string, "null"] estimatedSize: type: string + downloadBytes: + type: [integer, "null"] + description: >- + Compressed archive size for this host's + architecture, when the manifest records it. Null in + native (non-Docker) mode or when unmeasured. + installedBytes: + type: [integer, "null"] + description: >- + Extracted on-disk size for this host's + architecture, when the manifest records it. enablesTools: type: array items: diff --git a/apps/api/src/plugins/auth.ts b/apps/api/src/plugins/auth.ts index 17c61b58..6ab0ab59 100644 --- a/apps/api/src/plugins/auth.ts +++ b/apps/api/src/plugins/auth.ts @@ -51,7 +51,7 @@ export async function verifyPassword(password: string, stored: string): Promise< /** * Fixed-cost hash used to equalize login timing for usernames that don't - * exist (or have no local password). Computed once per process and cached -- + * exist (or have no local password). Computed once per process and cached; * without this, a login attempt for an unknown username returns as soon as * the user lookup misses, while a wrong password for a real user waits on a * full scrypt run. That gap is a timing side-channel an attacker can use to diff --git a/apps/api/src/routes/features.ts b/apps/api/src/routes/features.ts index a956c297..1afb0bee 100644 --- a/apps/api/src/routes/features.ts +++ b/apps/api/src/routes/features.ts @@ -74,6 +74,11 @@ function startInstall(bundleId: string, jobId: string): void { const installStartTime = Date.now(); void (async () => { + // Mark the install as started right away: a zero-percent frame claims the + // progress slot and clears any stale error left by a previous failed + // attempt of this bundle, so a retry never shows the old failure. + setInstallProgress(bundleId, { percent: 0, stage: "" }, null); + // Hold the venv lock across the whole install so no AI tool job loads // native libs from the venv while pip is rewriting them (that segfaults // the sidecar). This awaits any in-flight AI job before the installer @@ -87,6 +92,21 @@ function startInstall(bundleId: string, jobId: string): void { } }; + // A failed spawn fires BOTH "error" and "close", so both handlers funnel + // their teardown through this once-guard. Without it the second event + // would release the file lock and active slot that pump() just handed to + // the next queued bundle, letting two installers run into the same venv + // at once (the corruption the lock exists to prevent). + let finalized = false; + const finalizeOnce = (): boolean => { + if (finalized) return false; + finalized = true; + releaseVenvOnce(); + releaseInstallLock(); + clearActive(); + return true; + }; + const child = spawn(pythonPath, [scriptPath, bundleId, manifestPath, modelsDir], { stdio: ["ignore", "pipe", "pipe"], env: { @@ -139,15 +159,12 @@ function startInstall(bundleId: string, jobId: string): void { }); child.on("close", (code) => { - releaseVenvOnce(); - releaseInstallLock(); - clearActive(); - pump(); + if (!finalizeOnce()) return; if (code === 0) { invalidateCache(); shutdownDispatcher(); - setInstallProgress(null, null, null); + setInstallProgress(bundleId, null, null); updateSingleFileProgress({ jobId, phase: "complete", percent: 100, stage: "Complete" }); trackEvent(ANALYTICS_EVENTS.AI_BUNDLE_ACTION, { bundle_id: bundleId, @@ -195,16 +212,19 @@ function startInstall(bundleId: string, jobId: string): void { setInstallProgress(bundleId, null, errorMsg); updateSingleFileProgress({ jobId, phase: "failed", percent: 0, error: errorMsg }); } + + // Record the outcome BEFORE starting the next install, so the next + // bundle's first progress frame cannot race with (or be wiped by) this + // install's completion/failure bookkeeping. + pump(); }); child.on("error", (err) => { - releaseVenvOnce(); - releaseInstallLock(); - clearActive(); - pump(); + if (!finalizeOnce()) return; const errorMsg = `Failed to spawn install process: ${err.message}`; setInstallProgress(bundleId, null, errorMsg); updateSingleFileProgress({ jobId, phase: "failed", percent: 0, error: errorMsg }); + pump(); }); })(); } diff --git a/apps/api/src/routes/tools/compare.ts b/apps/api/src/routes/tools/compare.ts index c0cea3f2..fdfea4ee 100644 --- a/apps/api/src/routes/tools/compare.ts +++ b/apps/api/src/routes/tools/compare.ts @@ -48,16 +48,25 @@ export function registerCompare(app: FastifyInstance) { try { const imageHandler = inputHandlerFor("image"); - bufferA = ( - await imageHandler.prepare(bufferA, filenameA, { - scratchDir: tmpdir(), - }) - ).buffer; - bufferB = ( - await imageHandler.prepare(bufferB, filenameB, { - scratchDir: tmpdir(), - }) - ).buffer; + // Attribute validation failures to the specific upload: with two + // inputs, a bare "Invalid image: ..." does not tell the user which of + // their files was rejected. Restores the route's pre-migration + // "Invalid first/second image: ..." message contract. + const prepareInput = async (buffer: Buffer, filename: string, which: string) => { + try { + return (await imageHandler.prepare(buffer, filename, { scratchDir: tmpdir() })).buffer; + } catch (err) { + if (err instanceof InputValidationError) { + const message = err.message.startsWith("Invalid image") + ? err.message.replace("Invalid image", `Invalid ${which} image`) + : `${err.message} (${which} image)`; + throw new InputValidationError(message, err.statusCode, err.details); + } + throw err; + } + }; + bufferA = await prepareInput(bufferA, filenameA, "first"); + bufferB = await prepareInput(bufferB, filenameB, "second"); // Normalize both to same size for comparison const metaA = await sharp(bufferA).metadata(); diff --git a/apps/api/src/routes/tools/content-aware-resize.ts b/apps/api/src/routes/tools/content-aware-resize.ts index 0e2913b3..9de77096 100644 --- a/apps/api/src/routes/tools/content-aware-resize.ts +++ b/apps/api/src/routes/tools/content-aware-resize.ts @@ -12,6 +12,8 @@ import { sanitizeFilename } from "../../lib/filename.js"; import { decodeToSharpCompat, needsCliDecode } from "../../lib/format-decoders.js"; import { decodeHeic } from "../../lib/heic-converter.js"; import { putObject } from "../../lib/object-storage.js"; +import { InputValidationError } from "../../modality/contract.js"; +import { inputHandlerFor } from "../../modality/input-handler.js"; import { registerToolProcessFn } from "../tool-factory.js"; const settingsSchema = z.object({ @@ -59,37 +61,25 @@ export function registerContentAwareResize(app: FastifyInstance) { return reply.status(400).send({ error: "No image file provided" }); } - const validation = await validateImageBuffer(fileBuffer, filename); - if (!validation.valid) { - return reply.status(400).send({ error: `Invalid image: ${validation.reason}` }); - } - - // Decode HEIC/HEIF input (caire can't read HEIF containers) - if (validation.format === "heif") { - try { - fileBuffer = await decodeHeic(fileBuffer); - const ext = filename.match(/\.[^.]+$/)?.[0]; - if (ext) filename = `${filename.slice(0, -ext.length)}.png`; - } catch (err) { - return reply.status(422).send({ - error: "Failed to decode HEIC/HEIF file", - details: friendlyError(err instanceof Error ? err.message : String(err)), - }); - } - } - - // Decode CLI-decoded formats (RAW, TGA, PSD, EXR, HDR) - if (needsCliDecode(validation.format)) { - try { - fileBuffer = await decodeToSharpCompat(fileBuffer, validation.format); - const ext = filename.match(/\.[^.]+$/)?.[0]; - if (ext) filename = `${filename.slice(0, -ext.length)}.png`; - } catch (err) { - return reply.status(422).send({ - error: `Failed to decode ${validation.format} file`, - details: friendlyError(err instanceof Error ? err.message : String(err)), - }); + // Shared image input chain (validate, HEIC/RAW decode with filename + // rewrite, SVG sanitize, AVIF probe, autoOrient): the same handler the + // factory-based image routes use, replacing an inline copy that had + // already drifted (it never sanitized SVG or passed the file extension + // to the RAW decoder). + try { + const prepared = await inputHandlerFor("image").prepare(fileBuffer, filename, { + scratchDir: tmpdir(), + }); + fileBuffer = prepared.buffer; + filename = prepared.filename; + } catch (err) { + if (err instanceof InputValidationError) { + return reply.status(err.statusCode).send({ error: err.message, details: err.details }); } + return reply.status(422).send({ + error: "Failed to prepare image", + details: friendlyError(err instanceof Error ? err.message : String(err)), + }); } // Validate settings @@ -124,9 +114,6 @@ export function registerContentAwareResize(app: FastifyInstance) { "Starting content-aware resize", ); - // Auto-orient to fix EXIF rotation before seam carving - fileBuffer = await autoOrient(fileBuffer); - const jobId = randomUUID(); const scratchDir = join(tmpdir(), "snapotter-scratch", jobId); await mkdir(scratchDir, { recursive: true }); diff --git a/apps/api/src/routes/tools/embed-subtitles.ts b/apps/api/src/routes/tools/embed-subtitles.ts index aaa6e564..2e0ccbfc 100644 --- a/apps/api/src/routes/tools/embed-subtitles.ts +++ b/apps/api/src/routes/tools/embed-subtitles.ts @@ -64,8 +64,19 @@ export function registerEmbedSubtitles(app: FastifyInstance) { "0:v:0", "-map", "0:a?", + // The new subtitle is mapped BEFORE the source's existing subtitle + // tracks so it is always output subtitle stream s:0, which the + // language tag below targets. "-map", "1:0", + // Preserve subtitle tracks already in the source (re-encoded to the + // container's text codec, matching the old `-map 0` behavior). + // Data streams stay unmapped on purpose: MPEG data tracks such as + // teletext cannot be remuxed into mp4/mkv and fail the whole job. + "-map", + "0:s?", + // MKV can carry attachment streams (embedded fonts); MP4 cannot. + ...(toMp4 ? [] : ["-map", "0:t?"]), ...(reencodeInputStreams ? [...videoEncodeArgsForContainer(outExt), ...audioEncodeArgsForContainer(outExt)] : ["-c:v", "copy", "-c:a", "copy"]), diff --git a/apps/api/src/routes/tools/watermark-image.ts b/apps/api/src/routes/tools/watermark-image.ts index ff174c04..3e5b419d 100644 --- a/apps/api/src/routes/tools/watermark-image.ts +++ b/apps/api/src/routes/tools/watermark-image.ts @@ -1,16 +1,14 @@ import { randomUUID } from "node:crypto"; +import { tmpdir } from "node:os"; import type { FastifyInstance } from "fastify"; import sharp from "sharp"; import { z } from "zod"; -import { autoOrient } from "../../lib/auto-orient.js"; import { formatZodErrors } from "../../lib/errors.js"; -import { validateImageBuffer } from "../../lib/file-validation.js"; import { sanitizeFilename } from "../../lib/filename.js"; -import { decodeToSharpCompat, needsCliDecode } from "../../lib/format-decoders.js"; -import { decodeHeic } from "../../lib/heic-converter.js"; import { putObject } from "../../lib/object-storage.js"; import { resolveOutputFormat } from "../../lib/output-format.js"; -import { decompressSvgz, sanitizeSvg } from "../../lib/svg-sanitize.js"; +import { InputValidationError } from "../../modality/contract.js"; +import { inputHandlerFor } from "../../modality/input-handler.js"; const settingsSchema = z.object({ position: z @@ -81,86 +79,35 @@ export function registerWatermarkImage(app: FastifyInstance) { } try { - const valMain = await validateImageBuffer(mainBuffer, filename); - if (!valMain.valid) { - return reply.status(400).send({ error: `Invalid image: ${valMain.reason}` }); - } - if (valMain.format === "heif") { - try { - mainBuffer = await decodeHeic(mainBuffer); - } catch (err) { - return reply.status(422).send({ - error: "Failed to decode HEIC file. Ensure libheif-examples is installed.", - details: err instanceof Error ? err.message : String(err), - }); - } - } - if (needsCliDecode(valMain.format)) { - try { - const ext = filename.split(".").pop()?.toLowerCase(); - mainBuffer = await decodeToSharpCompat(mainBuffer, valMain.format, ext); - } catch { - try { - await sharp(mainBuffer).metadata(); - } catch (err) { - return reply.status(422).send({ - error: `Failed to decode ${valMain.format.toUpperCase()} file`, - details: err instanceof Error ? err.message : String(err), - }); - } - } - } - if (valMain.format === "svg") { - try { - mainBuffer = decompressSvgz(mainBuffer); - mainBuffer = sanitizeSvg(mainBuffer); - } catch (err) { - return reply.status(400).send({ - error: err instanceof Error ? err.message : "Invalid SVG", - }); - } - } - mainBuffer = await autoOrient(mainBuffer); + // Shared image input chain (validate, HEIC/RAW decode, SVG sanitize, + // AVIF probe, autoOrient), the same handler compare/vectorize/compose + // use. It also rewrites the filename extension after a decode so the + // output name and resolveOutputFormat below stay consistent. + const imageHandler = inputHandlerFor("image"); + const preparedMain = await imageHandler.prepare(mainBuffer, filename, { + scratchDir: tmpdir(), + }); + mainBuffer = preparedMain.buffer; + filename = preparedMain.filename; - const valWm = await validateImageBuffer(watermarkBuffer, watermarkFilename); - if (!valWm.valid) { - return reply.status(400).send({ error: `Invalid watermark image: ${valWm.reason}` }); - } - if (valWm.format === "heif") { - try { - watermarkBuffer = await decodeHeic(watermarkBuffer); - } catch (err) { - return reply.status(422).send({ - error: "Failed to decode watermark (HEIC). Ensure libheif-examples is installed.", - details: err instanceof Error ? err.message : String(err), - }); + try { + watermarkBuffer = ( + await imageHandler.prepare(watermarkBuffer, watermarkFilename, { + scratchDir: tmpdir(), + }) + ).buffer; + } catch (err) { + // Attribute validation failures to the watermark upload so the user + // knows which of the two files was rejected, preserving the route's + // established "Invalid watermark image: ..." message contract. + if (err instanceof InputValidationError) { + const message = err.message.startsWith("Invalid image") + ? err.message.replace("Invalid image", "Invalid watermark image") + : `${err.message} (watermark)`; + throw new InputValidationError(message, err.statusCode, err.details); } + throw err; } - if (needsCliDecode(valWm.format)) { - try { - watermarkBuffer = await decodeToSharpCompat(watermarkBuffer, valWm.format); - } catch { - try { - await sharp(watermarkBuffer).metadata(); - } catch (err) { - return reply.status(422).send({ - error: `Failed to decode watermark (${valWm.format.toUpperCase()})`, - details: err instanceof Error ? err.message : String(err), - }); - } - } - } - if (valWm.format === "svg") { - try { - watermarkBuffer = decompressSvgz(watermarkBuffer); - watermarkBuffer = sanitizeSvg(watermarkBuffer); - } catch (err) { - return reply.status(400).send({ - error: err instanceof Error ? err.message : "Invalid SVG (watermark)", - }); - } - } - watermarkBuffer = await autoOrient(watermarkBuffer); const mainImage = sharp(mainBuffer); const mainMeta = await mainImage.metadata(); @@ -242,6 +189,9 @@ export function registerWatermarkImage(app: FastifyInstance) { processedSize: result.length, }); } catch (err) { + if (err instanceof InputValidationError) { + return reply.status(err.statusCode).send({ error: err.message, details: err.details }); + } return reply.status(422).send({ error: "Processing failed", details: err instanceof Error ? err.message : "Image processing failed", diff --git a/apps/web/src/components/onboarding/usage-survey-overlay.tsx b/apps/web/src/components/onboarding/usage-survey-overlay.tsx index 88642cb7..5b806d6c 100644 --- a/apps/web/src/components/onboarding/usage-survey-overlay.tsx +++ b/apps/web/src/components/onboarding/usage-survey-overlay.tsx @@ -67,7 +67,13 @@ export function UsageSurveyOverlay() { if (!eligibleAuthState || !eligibleRoute) return; apiGet<{ settings: Record }>("/v1/settings") .then((data) => setSettings(data.settings)) - .catch(() => setSettings({})); + .catch(() => { + // Fail closed: without settings we cannot know whether the admin + // already answered, and showing the full-screen overlay while the + // API is unhealthy would soft-lock them (the dismiss/continue + // writes would fail against the same unhealthy API). Skipping the + // survey for this load is the cheap, recoverable outcome. + }); }, [eligibleAuthState, eligibleRoute]); const visible = diff --git a/apps/web/src/components/settings/ai-features-section.tsx b/apps/web/src/components/settings/ai-features-section.tsx index d0470c1d..7eaef092 100644 --- a/apps/web/src/components/settings/ai-features-section.tsx +++ b/apps/web/src/components/settings/ai-features-section.tsx @@ -285,7 +285,12 @@ function BundleCard({

{bundle.description} (~ {bundle.downloadBytes ? formatFileSize(bundle.downloadBytes) : bundle.estimatedSize} - {bundle.installedBytes ? `, ${formatFileSize(bundle.installedBytes)} on disk` : ""}) + {bundle.installedBytes + ? `, ${format(t.settings.aiFeatures.sizeOnDisk, { + size: formatFileSize(bundle.installedBytes), + })}` + : ""} + )

diff --git a/apps/web/src/stores/features-store.ts b/apps/web/src/stores/features-store.ts index 8cb1e630..a668dc3d 100644 --- a/apps/web/src/stores/features-store.ts +++ b/apps/web/src/stores/features-store.ts @@ -96,6 +96,14 @@ export const useFeaturesStore = create((set, get) => { maybeFinishInstallAll(); }; + const stopPolling = (bundleId: string) => { + const ref = pollRefs[bundleId]; + if (ref) { + clearInterval(ref); + delete pollRefs[bundleId]; + } + }; + const startPolling = (bundleId: string) => { if (pollRefs[bundleId]) return; pollRefs[bundleId] = setInterval(async () => { @@ -126,8 +134,7 @@ export const useFeaturesStore = create((set, get) => { } // Terminal: installed / error / not_installed. - clearInterval(pollRefs[bundleId]); - delete pollRefs[bundleId]; + stopPolling(bundleId); stopTracking(bundleId); onInstallSettled( bundleId, @@ -138,6 +145,13 @@ export const useFeaturesStore = create((set, get) => { }; const listenToProgress = (bundleId: string, jobId: string) => { + // One tracker per bundle: close any previous stream and stop any poll so + // a re-POST (which the server dedups to the same job) can never leave two + // live subscriptions whose terminal events each run the settle logic. + esRefs[bundleId]?.close(); + delete esRefs[bundleId]; + stopPolling(bundleId); + const es = new EventSource(`/api/v1/jobs/${jobId}/progress`); esRefs[bundleId] = es; @@ -277,7 +291,7 @@ export const useFeaturesStore = create((set, get) => { }, installBundle: async (bundleId: string) => { - // Always POST immediately -- the server owns the queue now, so a POST is + // Always POST immediately: the server owns the queue now, so a POST is // durable even if this tab closes. Optimistically show "installing"; the // response tells us whether it actually started or got queued. const errors = { ...get().errors }; @@ -294,16 +308,21 @@ export const useFeaturesStore = create((set, get) => { {}, ); if (result.queued) { - // Server queued it behind an active install; show the pill instead of - // a progress bar until the first progress frame arrives. + // Server queued it behind an active install; show the pill and poll + // for the transition instead of holding an SSE connection open. An + // EventSource per queued bundle would let Install All pin up to 7 + // connections for the whole run, exhausting the browser's + // per-origin limit on HTTP/1.1 and starving every other request. const installing = { ...get().installing }; delete installing[bundleId]; set({ installing, queued: get().queued.includes(bundleId) ? get().queued : [...get().queued, bundleId], }); + startPolling(bundleId); + } else { + listenToProgress(bundleId, result.jobId); } - listenToProgress(bundleId, result.jobId); } catch (err) { stopTracking(bundleId); @@ -344,7 +363,12 @@ export const useFeaturesStore = create((set, get) => { }, installAll: async () => { - const pending = get().bundles.filter((b) => b.status !== "installed"); + // Skip bundles the server is already installing or holding in its + // queue: re-POSTing them just dedups server-side and used to leave a + // second progress subscription racing the first one's terminal events. + const pending = get().bundles.filter( + (b) => b.status !== "installed" && b.status !== "installing" && b.status !== "queued", + ); if (pending.length === 0) return; // Mark every pending bundle up front so the run never looks "drained" diff --git a/packages/ai/python/install_feature.py b/packages/ai/python/install_feature.py index f3e94fee..c2a1c772 100644 --- a/packages/ai/python/install_feature.py +++ b/packages/ai/python/install_feature.py @@ -51,7 +51,7 @@ def detect_arch() -> str: Only two archive variants are currently published to the bundle repo: 'amd64-gpu' and 'arm64-cpu' (see deepsafe/feature-bundles). There is no CPU-only amd64 variant yet, so amd64 hosts always resolve to 'amd64-gpu' - even when no GPU is present -- this downloads working CUDA-capable + even when no GPU is present: this downloads working CUDA-capable packages, just larger than a CPU-only host strictly needs. Do not change this to branch on GPU presence without first publishing an 'amd64-cpu' archive for every bundle; requesting a key that doesn't exist in the @@ -97,7 +97,7 @@ def estimate_extracted(compressed: int, extracted: int) -> int: extractedSize (0), the budget would otherwise collapse to just the compressed size and under-reserve for the extracted payload; fall back to a conservative 3x of compressed (measured extracted/compressed ratios reach - ~3x). This is only the early sanity bail -- the accurate guard is the + ~3x). This is only the early sanity bail; the accurate guard is the real-on-disk re-check just before the destructive venv write.""" return extracted if extracted > 0 else compressed * 3 @@ -499,22 +499,23 @@ def main() -> None: # -- Disk re-check before the first destructive venv write -- # The upfront check ran before the download and used an estimate; now the # payload is really on disk, so measure it and verify there's room to place - # it before we start writing into the venv. On the same filesystem the move - # is a rename (no extra space needed, just a safety floor); across - # filesystems it is a copy that transiently needs the payload's size again. - # Running here (after the local/remote branches merge) also covers the - # offline-import path, which skipped the upfront check entirely. - staging_real = dir_size(staging_dir) - if same_filesystem(staging_dir, venv_path): - recheck_needed = 1024 ** 3 # 1 GB floor for fixups / installed.json / slack - else: - recheck_needed = staging_real + 1024 ** 3 - check_disk_space(ai_dir, recheck_needed) + # it before we start writing into the venv. Running here (after the + # local/remote branches merge) also covers the offline-import path, which + # skipped the upfront check entirely. Each budget is checked against the + # filesystem the bytes actually land on: when the venv lives on a + # different filesystem than staging, the site-packages payload is COPIED + # onto the venv's disk, so that disk (not ai_dir's) must hold it. Models + # stay under ai_dir either way, moving by rename. + disk_floor = 1024 ** 3 # 1 GB for fixups / installed.json / slack + staging_sp = os.path.join(staging_dir, "site-packages") + if not same_filesystem(staging_dir, venv_path): + sp_bytes = dir_size(staging_sp) if os.path.isdir(staging_sp) else 0 + check_disk_space(venv_path, sp_bytes + disk_floor) + check_disk_space(ai_dir, disk_floor) # -- Move site-packages -- emit_progress(92, "Installing packages...") site_packages_dir = get_site_packages_dir(venv_path) - staging_sp = os.path.join(staging_dir, "site-packages") try: if os.path.isdir(staging_sp) and site_packages_dir: diff --git a/packages/ai/src/bridge.ts b/packages/ai/src/bridge.ts index 88ce6cf6..9bdbb53d 100644 --- a/packages/ai/src/bridge.ts +++ b/packages/ai/src/bridge.ts @@ -105,6 +105,8 @@ interface PendingRequest { reject: (err: Error) => void; onProgress?: ProgressCallback; stderrLines: string[]; + /** Which child generation this request was written to (see startChild). */ + generation: number; } // Crash recovery constants @@ -133,11 +135,24 @@ export class PythonDispatcher { private childFailed = false; private gpuAvail = false; private pending = new Map(); - private stdoutBuf = ""; private crashes = 0; private lastCrashTs = 0; private backoffEnd = 0; - private shuttingDown = false; + /** + * Monotonic child counter. Each spawned child and every request written to + * it carry the generation current at spawn time, so a stale child's late + * close/error events (SIGTERM delivery can lag a replacement spawn) only + * ever touch their own generation's pending requests. + */ + private generation = 0; + /** + * Children we SIGTERMed on purpose (shutdown/reload). Tracked per child + * rather than as an instance-wide flag: an instance flag reset by the next + * spawn would let the stale child's close event record a phantom crash and + * null out the fresh child. The request-timeout kill path deliberately does + * NOT add to this set, so a genuinely hung script still counts as a crash. + */ + private stoppedChildren = new WeakSet(); constructor(opts: { profile: "ai" | "docs" }) { this.profile = opts.profile; @@ -173,9 +188,19 @@ export class PythonDispatcher { ); } + /** Reject and drop the pending requests written to one child generation. */ + private rejectPendingForGeneration(generation: number, message: string): void { + for (const [id, req] of this.pending.entries()) { + if (req.generation !== generation) continue; + req.reject(new Error(message)); + this.pending.delete(id); + } + } + private startChild(): ChildProcess | null { if (this.childFailed) return null; - this.shuttingDown = false; + this.generation++; + const gen = this.generation; try { const proc = spawn(getPythonPath(), [resolve(PYTHON_DIR, "dispatcher.py")], { @@ -188,23 +213,23 @@ export class PythonDispatcher { console.error( `[bridge] Dispatcher stdin pipe broken (${err.code}), rejecting pending requests`, ); - for (const [id, req] of this.pending.entries()) { - req.reject(new Error("Python dispatcher stdin closed unexpectedly")); - this.pending.delete(id); + this.rejectPendingForGeneration(gen, "Python dispatcher stdin closed unexpectedly"); + // An intentional shutdown() ends stdin then SIGTERMs the child, + // which can surface here as an EPIPE/ERR_STREAM_DESTROYED. That is + // not a crash: counting it would let repeated legitimate restarts + // (shutdownDispatcher() runs after every AI bundle install) trip + // the crash limit and permanently disable the dispatcher. Guard + // mirrors the "close" handler below. + if (!this.stoppedChildren.has(proc)) this.recordCrash(); + if (this.child === proc) { + this.child = null; + this.childReady = false; } - // An intentional shutdown() ends stdin then SIGTERMs the child, which - // can surface here as an EPIPE/ERR_STREAM_DESTROYED. That is not a - // crash -- counting it would let repeated legitimate restarts (e.g. - // shutdownDispatcher() on every AI bundle install) trip the crash - // limit and permanently disable the dispatcher. Guard mirrors the - // "close" handler below. - if (!this.shuttingDown) this.recordCrash(); - this.child = null; - this.childReady = false; } }); let stderrBuf = ""; + let stdoutBuf = ""; proc.stderr?.on("data", (chunk: Buffer) => { stderrBuf += chunk.toString(); @@ -218,21 +243,24 @@ export class PythonDispatcher { try { const parsed = JSON.parse(trimmed); - // Readiness signal + // Readiness signal. Ignore it from a superseded child so a stale + // process can't mark a not-yet-ready replacement as ready. if (parsed.ready === true) { - this.childReady = true; - this.gpuAvail = parsed.gpu === true; - this.crashes = 0; - console.log(`[bridge] Python dispatcher ready (GPU: ${parsed.gpu === true})`); + if (this.child === proc) { + this.childReady = true; + this.gpuAvail = parsed.gpu === true; + this.crashes = 0; + console.log(`[bridge] Python dispatcher ready (GPU: ${parsed.gpu === true})`); + } continue; } // Progress event - route to the currently active request if (typeof parsed.progress === "number" && typeof parsed.stage === "string") { - // Progress goes to all pending requests (only one should be active at a time - // since Python processes synchronously) + // Progress goes to this child's pending requests (only one + // should be active at a time since Python processes synchronously) for (const req of this.pending.values()) { - req.onProgress?.(parsed.progress, parsed.stage); + if (req.generation === gen) req.onProgress?.(parsed.progress, parsed.stage); } } } catch { @@ -242,16 +270,16 @@ export class PythonDispatcher { console.log(`[python] ${trimmed}`); } for (const req of this.pending.values()) { - req.stderrLines.push(trimmed); + if (req.generation === gen) req.stderrLines.push(trimmed); } } } }); proc.stdout?.on("data", (chunk: Buffer) => { - this.stdoutBuf += chunk.toString(); - const lines = this.stdoutBuf.split("\n"); - this.stdoutBuf = lines.pop() ?? ""; + stdoutBuf += chunk.toString(); + const lines = stdoutBuf.split("\n"); + stdoutBuf = lines.pop() ?? ""; for (const line of lines) { const trimmed = line.trim(); @@ -292,29 +320,27 @@ export class PythonDispatcher { console.error(`[bridge] Dispatcher error: ${err.message} (code: ${err.code})`); if (err.code === "ENOENT") { this.childFailed = true; - } else if (!this.shuttingDown) { + } else if (!this.stoppedChildren.has(proc)) { // Skip crash accounting when we initiated the teardown (shutdown() - // sets shuttingDown before killing the child); mirrors "close". + // marks the child stopped before killing it); mirrors "close". this.recordCrash(); } - for (const [id, req] of this.pending.entries()) { - req.reject(new Error(extractPythonError(err))); - this.pending.delete(id); + this.rejectPendingForGeneration(gen, extractPythonError(err)); + if (this.child === proc) { + this.child = null; + this.childReady = false; } - this.child = null; - this.childReady = false; }); proc.on("close", (code) => { - for (const [id, req] of this.pending.entries()) { - req.reject(new Error("Python dispatcher exited unexpectedly")); - this.pending.delete(id); - } - if (code !== 0 && !this.shuttingDown) { + this.rejectPendingForGeneration(gen, "Python dispatcher exited unexpectedly"); + if (code !== 0 && !this.stoppedChildren.has(proc)) { this.recordCrash(); } - this.child = null; - this.childReady = false; + if (this.child === proc) { + this.child = null; + this.childReady = false; + } }); return proc; @@ -378,6 +404,9 @@ export class PythonDispatcher { reject: wrappedReject, onProgress: options.onProgress, stderrLines: [], + // getChild() above either reused or just spawned the child this + // request is written to, so the current generation is its generation. + generation: this.generation, }); const msg: Record = { id, script: scriptName.replace(".py", ""), args }; @@ -541,7 +570,7 @@ export class PythonDispatcher { */ shutdown(): void { if (this.child && !this.child.killed) { - this.shuttingDown = true; + this.stoppedChildren.add(this.child); this.child.stdin?.end(); this.child.kill("SIGTERM"); this.child = null; diff --git a/packages/image-engine/src/operations/auto-enhance.ts b/packages/image-engine/src/operations/auto-enhance.ts index 9f97234d..945c09fa 100644 --- a/packages/image-engine/src/operations/auto-enhance.ts +++ b/packages/image-engine/src/operations/auto-enhance.ts @@ -10,7 +10,7 @@ import type { } from "../types.js"; /** - * Above this pixel count, CLAHE is skipped in applyCorrections() -- see the + * Above this pixel count, CLAHE is skipped in applyCorrections(); see the * comment at its call site for why. */ const MAX_CLAHE_PIXELS = 16_000_000; @@ -230,7 +230,7 @@ export function applyCorrections( let result = image; // Tracks whether .clahe() actually ran (not just whether the toggle allowed - // it) -- Step 5 below applies a compensation boost keyed off this, and it + // it). Step 5 below applies a compensation boost keyed off this, and it // needs to stay correct now that CLAHE can also be skipped by image size. let claheApplied = false; @@ -238,7 +238,7 @@ export function applyCorrections( // maxSlope must be an integer (Sharp requirement); skip for tiny images. // CLAHE's cost scales with total pixel count regardless of tile size (tile // size only bounds granularity, not the per-pixel histogram/interpolation - // work), so it's also skipped above MAX_CLAHE_PIXELS -- a 5504x3672 (20MP) + // work), so it's also skipped above MAX_CLAHE_PIXELS: a 5504x3672 (20MP) // real-world RAW photo measured 40+ seconds in this step alone versus ~1s // for every other correction combined. The other six corrections below // still apply at full resolution regardless of size. diff --git a/packages/shared/src/i18n/ar.ts b/packages/shared/src/i18n/ar.ts index 0c62371e..24bbb1fc 100644 --- a/packages/shared/src/i18n/ar.ts +++ b/packages/shared/src/i18n/ar.ts @@ -3475,6 +3475,7 @@ export const ar: TranslationKeys = { description: "إدارة حزم نماذج AI لمعالجة الملفات المتقدمة.", installAll: "تثبيت الكل", diskUsage: "استخدام القرص: {size}", + sizeOnDisk: "{size} على القرص", installed: "مُثبَّت", notInstalled: "غير مُثبَّت", queued: "في قائمة الانتظار", diff --git a/packages/shared/src/i18n/de.ts b/packages/shared/src/i18n/de.ts index db0d02e2..88834ef5 100644 --- a/packages/shared/src/i18n/de.ts +++ b/packages/shared/src/i18n/de.ts @@ -3512,6 +3512,7 @@ export const de: TranslationKeys = { description: "AI-Modellpakete für erweiterte Dateiverarbeitung verwalten.", installAll: "Alle installieren", diskUsage: "Speicherplatznutzung: {size}", + sizeOnDisk: "{size} auf der Festplatte", installed: "Installiert", notInstalled: "Nicht installiert", queued: "In Warteschlange", diff --git a/packages/shared/src/i18n/en.ts b/packages/shared/src/i18n/en.ts index 27d89592..34f68a8a 100644 --- a/packages/shared/src/i18n/en.ts +++ b/packages/shared/src/i18n/en.ts @@ -3425,6 +3425,7 @@ export const en = { description: "Manage AI model bundles for advanced file processing.", installAll: "Install All", diskUsage: "Disk usage: {size}", + sizeOnDisk: "{size} on disk", installed: "Installed", notInstalled: "Not installed", queued: "Queued", diff --git a/packages/shared/src/i18n/es.ts b/packages/shared/src/i18n/es.ts index 70a3b710..45802116 100644 --- a/packages/shared/src/i18n/es.ts +++ b/packages/shared/src/i18n/es.ts @@ -3492,6 +3492,7 @@ export const es: TranslationKeys = { description: "Gestiona paquetes de modelos AI para procesamiento avanzado de archivos.", installAll: "Instalar todo", diskUsage: "Uso del disco: {size}", + sizeOnDisk: "{size} en disco", installed: "Instalado", notInstalled: "No instalado", queued: "En cola", diff --git a/packages/shared/src/i18n/fr.ts b/packages/shared/src/i18n/fr.ts index f5384a78..769b05ba 100644 --- a/packages/shared/src/i18n/fr.ts +++ b/packages/shared/src/i18n/fr.ts @@ -3516,6 +3516,7 @@ export const fr: TranslationKeys = { description: "Gérez les paquets de modèles AI pour le traitement avancé de fichiers.", installAll: "Tout installer", diskUsage: "Utilisation du disque : {size}", + sizeOnDisk: "{size} sur le disque", installed: "Installé", notInstalled: "Non installé", queued: "En file d'attente", diff --git a/packages/shared/src/i18n/hi.ts b/packages/shared/src/i18n/hi.ts index 142cdbf4..d7369204 100644 --- a/packages/shared/src/i18n/hi.ts +++ b/packages/shared/src/i18n/hi.ts @@ -3303,6 +3303,7 @@ export const hi: TranslationKeys = { description: "उन्नत फ़ाइल प्रोसेसिंग के लिए AI मॉडल बंडल प्रबंधित करें।", installAll: "सभी इंस्टॉल करें", diskUsage: "डिस्क उपयोग: {size}", + sizeOnDisk: "डिस्क पर {size}", installed: "इंस्टॉल किया गया", notInstalled: "इंस्टॉल नहीं है", queued: "कतार में", diff --git a/packages/shared/src/i18n/id.ts b/packages/shared/src/i18n/id.ts index bf971cc0..4a9582ec 100644 --- a/packages/shared/src/i18n/id.ts +++ b/packages/shared/src/i18n/id.ts @@ -3494,6 +3494,7 @@ export const id: TranslationKeys = { description: "Kelola bundel model AI untuk pemrosesan file tingkat lanjut.", installAll: "Instal Semua", diskUsage: "Penggunaan disk: {size}", + sizeOnDisk: "{size} di disk", installed: "Terinstal", notInstalled: "Belum terinstal", queued: "Dalam antrean", diff --git a/packages/shared/src/i18n/it.ts b/packages/shared/src/i18n/it.ts index e7da74d6..cc5363db 100644 --- a/packages/shared/src/i18n/it.ts +++ b/packages/shared/src/i18n/it.ts @@ -3506,6 +3506,7 @@ export const it: TranslationKeys = { description: "Gestisci pacchetti di modelli di IA per l'elaborazione avanzata dei file.", installAll: "Installa tutto", diskUsage: "Utilizzo del disco: {size}", + sizeOnDisk: "{size} su disco", installed: "Installato", notInstalled: "Non installato", queued: "Code", diff --git a/packages/shared/src/i18n/ja.ts b/packages/shared/src/i18n/ja.ts index 2cff15c8..bae53c7d 100644 --- a/packages/shared/src/i18n/ja.ts +++ b/packages/shared/src/i18n/ja.ts @@ -3445,6 +3445,7 @@ export const ja: TranslationKeys = { description: "高度なファイル処理のためのAIモデルバンドルを管理します。", installAll: "すべてインストール", diskUsage: "ディスク使用量: {size}", + sizeOnDisk: "ディスク上 {size}", installed: "インストール済み", notInstalled: "未インストール", queued: "待機中", diff --git a/packages/shared/src/i18n/ko.ts b/packages/shared/src/i18n/ko.ts index fe97ae8f..422813ec 100644 --- a/packages/shared/src/i18n/ko.ts +++ b/packages/shared/src/i18n/ko.ts @@ -3429,6 +3429,7 @@ export const ko: TranslationKeys = { description: "고급 파일 처리를 위한 AI 모델 번들을 관리합니다.", installAll: "모두 설치", diskUsage: "디스크 사용량: {size}", + sizeOnDisk: "디스크에 {size}", installed: "설치됨", notInstalled: "설치되지 않음", queued: "대기 중", diff --git a/packages/shared/src/i18n/nl.ts b/packages/shared/src/i18n/nl.ts index 1e61e6cc..2e2937fe 100644 --- a/packages/shared/src/i18n/nl.ts +++ b/packages/shared/src/i18n/nl.ts @@ -3502,6 +3502,7 @@ export const nl: TranslationKeys = { description: "Beheer AI-modelbundels voor geavanceerde bestandsverwerking.", installAll: "Alles installeren", diskUsage: "Schijfgebruik: {size}", + sizeOnDisk: "{size} op schijf", installed: "Geïnstalleerd", notInstalled: "Niet geïnstalleerd", queued: "In wachtrij", diff --git a/packages/shared/src/i18n/pl.ts b/packages/shared/src/i18n/pl.ts index c5b20d1d..9bb9ddae 100644 --- a/packages/shared/src/i18n/pl.ts +++ b/packages/shared/src/i18n/pl.ts @@ -3507,6 +3507,7 @@ export const pl: TranslationKeys = { description: "Zarządzanie pakietami modeli AI do zaawansowanego przetwarzania plików.", installAll: "Zainstaluj wszystkie", diskUsage: "Wykorzystanie dysku: {size}", + sizeOnDisk: "{size} na dysku", installed: "Zainstalowane", notInstalled: "Niezainstalowane", queued: "W kolejce", diff --git a/packages/shared/src/i18n/pt-BR.ts b/packages/shared/src/i18n/pt-BR.ts index 38c99581..1a83c570 100644 --- a/packages/shared/src/i18n/pt-BR.ts +++ b/packages/shared/src/i18n/pt-BR.ts @@ -3500,6 +3500,7 @@ export const ptBR: TranslationKeys = { description: "Gerencie pacotes de modelos de IA para processamento avançado de arquivos.", installAll: "Instalar tudo", diskUsage: "Uso do disco: {size}", + sizeOnDisk: "{size} em disco", installed: "Instalado", notInstalled: "Não instalado", queued: "Na fila", diff --git a/packages/shared/src/i18n/ru.ts b/packages/shared/src/i18n/ru.ts index 3dbe54c4..2e79205f 100644 --- a/packages/shared/src/i18n/ru.ts +++ b/packages/shared/src/i18n/ru.ts @@ -3497,6 +3497,7 @@ export const ru: TranslationKeys = { description: "Управление пакетами AI-моделей для расширенной обработки файлов.", installAll: "Установить все", diskUsage: "Использование диска: {size}", + sizeOnDisk: "{size} на диске", installed: "Установлено", notInstalled: "Не установлено", queued: "В очереди", diff --git a/packages/shared/src/i18n/sv.ts b/packages/shared/src/i18n/sv.ts index aa86e3d6..00699109 100644 --- a/packages/shared/src/i18n/sv.ts +++ b/packages/shared/src/i18n/sv.ts @@ -3492,6 +3492,7 @@ export const sv: TranslationKeys = { description: "Hantera AI-modellpaket för avancerad filbehandling.", installAll: "Installera alla", diskUsage: "Diskanvändning: {size}", + sizeOnDisk: "{size} på disk", installed: "Installerad", notInstalled: "Inte installerad", queued: "I kö", diff --git a/packages/shared/src/i18n/th.ts b/packages/shared/src/i18n/th.ts index 1e11a49f..f4801f08 100644 --- a/packages/shared/src/i18n/th.ts +++ b/packages/shared/src/i18n/th.ts @@ -3456,6 +3456,7 @@ export const th: TranslationKeys = { description: "จัดการชุดโมเดล AI สำหรับการประมวลผลไฟล์ขั้นสูง", installAll: "ติดตั้งทั้งหมด", diskUsage: "การใช้พื้นที่ดิสก์: {size}", + sizeOnDisk: "{size} บนดิสก์", installed: "ติดตั้งแล้ว", notInstalled: "ยังไม่ได้ติดตั้ง", queued: "อยู่ในคิว", diff --git a/packages/shared/src/i18n/tr.ts b/packages/shared/src/i18n/tr.ts index f22cd682..b4505cde 100644 --- a/packages/shared/src/i18n/tr.ts +++ b/packages/shared/src/i18n/tr.ts @@ -3500,6 +3500,7 @@ export const tr: TranslationKeys = { description: "Gelişmiş dosya işleme için AI model paketlerini yönetin.", installAll: "Tümünü Kur", diskUsage: "Disk kullanımı: {size}", + sizeOnDisk: "diskte {size}", installed: "Kurulu", notInstalled: "Kurulu değil", queued: "Sırada", diff --git a/packages/shared/src/i18n/uk.ts b/packages/shared/src/i18n/uk.ts index ee049e3b..c697d806 100644 --- a/packages/shared/src/i18n/uk.ts +++ b/packages/shared/src/i18n/uk.ts @@ -3497,6 +3497,7 @@ export const uk: TranslationKeys = { description: "Керування пакетами AI-моделей для розширеної обробки файлів.", installAll: "Встановити все", diskUsage: "Використання диска: {size}", + sizeOnDisk: "{size} на диску", installed: "Встановлено", notInstalled: "Не встановлено", queued: "У черзі", diff --git a/packages/shared/src/i18n/vi.ts b/packages/shared/src/i18n/vi.ts index e2fbbf26..301eb542 100644 --- a/packages/shared/src/i18n/vi.ts +++ b/packages/shared/src/i18n/vi.ts @@ -3492,6 +3492,7 @@ export const vi: TranslationKeys = { description: "Quản lý các gói mô hình AI cho xử lý tệp nâng cao.", installAll: "Cài đặt tất cả", diskUsage: "Dung lượng đĩa: {size}", + sizeOnDisk: "{size} trên ổ đĩa", installed: "Đã cài đặt", notInstalled: "Chưa cài đặt", queued: "Đang chờ", diff --git a/packages/shared/src/i18n/zh-CN.ts b/packages/shared/src/i18n/zh-CN.ts index 686eac70..7aea888b 100644 --- a/packages/shared/src/i18n/zh-CN.ts +++ b/packages/shared/src/i18n/zh-CN.ts @@ -3243,6 +3243,7 @@ export const zhCN: TranslationKeys = { description: "管理用于高级文件处理的 AI 模型包。", installAll: "全部安装", diskUsage: "磁盘使用量：{size}", + sizeOnDisk: "占用磁盘 {size}", installed: "已安装", notInstalled: "未安装", queued: "排队中", diff --git a/packages/shared/src/i18n/zh-TW.ts b/packages/shared/src/i18n/zh-TW.ts index ca3d5089..48e5c809 100644 --- a/packages/shared/src/i18n/zh-TW.ts +++ b/packages/shared/src/i18n/zh-TW.ts @@ -3241,6 +3241,7 @@ export const zhTW: TranslationKeys = { description: "管理用於進階檔案處理的AI模型套件。", installAll: "全部安裝", diskUsage: "磁碟使用量：{size}", + sizeOnDisk: "佔用磁碟 {size}", installed: "已安裝", notInstalled: "未安裝", queued: "等待中", diff --git a/tests/e2e-docker/feature-lifecycle.spec.ts b/tests/e2e-docker/feature-lifecycle.spec.ts index ba110b56..0e079129 100644 --- a/tests/e2e-docker/feature-lifecycle.spec.ts +++ b/tests/e2e-docker/feature-lifecycle.spec.ts @@ -316,24 +316,33 @@ test.describe("Install lifecycle - face-detection", () => { }); test("install of different bundle while one is active is queued (202)", async ({ request }) => { - test.setTimeout(900_000); + // The drain below waits out two full installs, each with the helper's + // 600s default budget, so the test timeout must cover both plus slack. + test.setTimeout(1_500_000); const status = await getBundleStatus(request, "face-detection"); if (status !== "installing") return; // fast install already finished; nothing to assert const headers = await authHeaders(request); - const res = await request.post(`${API}/api/v1/admin/features/ocr/install`, { headers }); + // Queue transcription: the smallest bundle (~0.5 GB archive) that no + // other test here depends on. There is no cancel API for a queued + // install, so the drain below really downloads it; queueing ocr here + // used to pull a ~6 GB archive just to assert queued=true. + const res = await request.post(`${API}/api/v1/admin/features/transcription/install`, { + headers, + }); expect(res.status()).toBe(202); expect((await res.json()).queued).toBe(true); - const ocr = await getBundle(request, "ocr"); - expect(["queued", "installing"]).toContain(ocr.status); + const queuedBundle = await getBundle(request, "transcription"); + expect(["queued", "installing"]).toContain(queuedBundle.status); - // ocr auto-starts once face-detection finishes. Drain and uninstall it so - // the rest of the serial suite runs against a clean lock/state. + // transcription auto-starts once face-detection finishes. Drain and + // uninstall it so the rest of the serial suite runs against a clean + // lock/state. await waitForInstallComplete(request, "face-detection"); - await waitForInstallComplete(request, "ocr", 900_000); - await request.post(`${API}/api/v1/admin/features/ocr/uninstall`, { headers }); - expect(await getBundleStatus(request, "ocr")).toBe("not_installed"); + await waitForInstallComplete(request, "transcription"); + await request.post(`${API}/api/v1/admin/features/transcription/uninstall`, { headers }); + expect(await getBundleStatus(request, "transcription")).toBe("not_installed"); }); test("after install completes, status is installed with version", async ({ request }) => { diff --git a/tests/integration/platform/auth-edge-cases.test.ts b/tests/integration/platform/auth-edge-cases.test.ts index ad498c83..b574684c 100644 --- a/tests/integration/platform/auth-edge-cases.test.ts +++ b/tests/integration/platform/auth-edge-cases.test.ts @@ -112,7 +112,7 @@ describe("Login failures", () => { // hash (scrypt), while "wrong password for a real user" always pays the // scrypt cost. That gap lets an attacker enumerate valid usernames purely // from response timing even though the status code and body are identical. - // See getDummyHash() in apps/api/src/plugins/auth.ts -- it equalizes cost + // See getDummyHash() in apps/api/src/plugins/auth.ts; it equalizes cost // by running verifyPassword against a dummy hash on the unknown-user path. const SAMPLES = 10; const median = (values: number[]) => { diff --git a/tests/integration/platform/feature-install-queue.test.ts b/tests/integration/platform/feature-install-queue.test.ts index be033519..d475d48c 100644 --- a/tests/integration/platform/feature-install-queue.test.ts +++ b/tests/integration/platform/feature-install-queue.test.ts @@ -5,7 +5,7 @@ * The installer child process (spawn) and the venv lock (@snapotter/ai) are * mocked so no real Python runs: spawn returns a controllable fake child we can * drive with emit("close"). This lets us assert the route contract - * deterministically -- a second concurrent install is queued (202 { queued: + * deterministically: a second concurrent install is queued (202 { queued: * true }) instead of rejected, the next queued bundle auto-starts when the * running one finishes, and a bundle queued while an import holds the lock * starts once the import route releases it. diff --git a/tests/integration/tools/data/extract-zip.test.ts b/tests/integration/tools/data/extract-zip.test.ts index 1f68c3c0..845a1e36 100644 --- a/tests/integration/tools/data/extract-zip.test.ts +++ b/tests/integration/tools/data/extract-zip.test.ts @@ -135,7 +135,7 @@ describe("extract-zip (pure JS, no skipIf)", () => { // Same binary-patch technique as the "../evil.txt" case above, but with a // multi-segment ".." path to make sure the segment-splitting check // (name.split(/[/\\]/).some(s => s === "..")) isn't fooled by extra - // "../" hops -- only the first ".." segment mattering would be a bug. + // "../" hops; only the first ".." segment mattering would be a bug. const zip = new AdmZip(); const traversalName = "../../../etc/passphrase-lol"; const placeholder = "X".repeat(traversalName.length); @@ -157,7 +157,7 @@ describe("extract-zip (pure JS, no skipIf)", () => { }, 30_000); it("rejects a zip with an absolute Unix path entry (/etc/passwd) with 400", async () => { - // Absolute paths bypass ".." detection entirely -- extract-zip must reject + // Absolute paths bypass ".." detection entirely; extract-zip must reject // entry names starting with "/" on their own (preValidate's // name.startsWith("/") branch), independent of the ".." segment check. const zip = new AdmZip(); diff --git a/tests/qa/api-sweep.mts b/tests/qa/api-sweep.mts index c08524f9..6236a3a6 100644 --- a/tests/qa/api-sweep.mts +++ b/tests/qa/api-sweep.mts @@ -640,7 +640,7 @@ async function main() { format: ext, status: statusCode, outputOk: false, - note: `BUG: custom body route returned ${statusCode} -- ${body.slice(0, 300)}`, + note: `BUG: custom body route returned ${statusCode}. ${body.slice(0, 300)}`, }; results.push(r); bugs.push(r); @@ -673,7 +673,7 @@ async function main() { outputOk: verification.ok, note: verification.ok ? `pass: ${verification.detail}` - : `BUG: corrupt success -- ${verification.detail}`, + : `BUG: corrupt success. ${verification.detail}`, }; results.push(r); if (verification.ok) { @@ -691,7 +691,7 @@ async function main() { format: ext, status: "network-error", outputOk: false, - note: `BUG: custom body request failed -- ${msg.slice(0, 200)}`, + note: `BUG: custom body request failed. ${msg.slice(0, 200)}`, }; results.push(r); bugs.push(r); @@ -817,7 +817,7 @@ async function main() { } catch {} const msg = parsed.error || parsed.details || body.slice(0, 200); const fullMsg = - [parsed.error, parsed.details].filter(Boolean).join(" -- ") || body.slice(0, 300); + [parsed.error, parsed.details].filter(Boolean).join(" | ") || body.slice(0, 300); // Check if this format is in the tool's own acceptedInputs const isSelfFormat = tool.acceptedInputs.includes(ext); @@ -993,7 +993,7 @@ async function main() { outputOk: verification.ok, note: verification.ok ? `pass: ${verification.detail}` - : `BUG: corrupt success -- ${verification.detail}`, + : `BUG: corrupt success. ${verification.detail}`, }; results.push(r); diff --git a/tests/qa/docker-compose.qa.yml b/tests/qa/docker-compose.qa.yml index 66bc5908..d19d5d34 100644 --- a/tests/qa/docker-compose.qa.yml +++ b/tests/qa/docker-compose.qa.yml @@ -8,8 +8,10 @@ # Project/container names default to snapotter-qa. Two sessions on the same host # running this file verbatim at the same time will silently steal each other's # container (last `up` wins, no error) since container_name is fixed rather than -# derived from the project name. If you need a second concurrent stack, override: -# QA_PROJECT_NAME=snapotter-qa-2 docker compose -f tests/qa/docker-compose.qa.yml up -d +# derived from the project name. A second concurrent stack also needs its own +# host port (the default stack holds 13499), so override both: +# QA_PROJECT_NAME=snapotter-qa-2 QA_APP_PORT=13498 \ +# docker compose -f tests/qa/docker-compose.qa.yml up -d name: ${QA_PROJECT_NAME:-snapotter-qa} services: @@ -17,7 +19,7 @@ services: image: snapotter/snapotter:latest container_name: ${QA_PROJECT_NAME:-snapotter-qa} ports: - - "13499:1349" + - "${QA_APP_PORT:-13499}:1349" volumes: - qa-data:/data - qa-workspace:/tmp/workspace diff --git a/tests/qa/install-ai-bundles-ui.mts b/tests/qa/install-ai-bundles-ui.mts index 8a4e4276..a1166418 100644 --- a/tests/qa/install-ai-bundles-ui.mts +++ b/tests/qa/install-ai-bundles-ui.mts @@ -1,7 +1,7 @@ // Drives the REAL in-app AI bundle install flow (Settings > AI Features > Install // All) against a running Docker container, exactly as a user would click through // it. This is the primary AI-install path per docs/prompts/engineering/QA_PROMPT.md -// Phase 2 -- the curl-based /api/v1/admin/features//install route is a +// Phase 2: the curl-based /api/v1/admin/features//install route is a // verification/fallback path only, never the primary install. // // This script only KICKS OFF the install and captures pre/mid-install evidence; the @@ -47,11 +47,11 @@ async function main() { console.log(" already authenticated (no login form shown)"); } - // Bail loudly if we land on the forced password-change screen -- the caller + // Bail loudly if we land on the forced password-change screen; the caller // should set SKIP_MUST_CHANGE_PASSWORD=true for automated runs. if (page.url().includes("/change-password")) { throw new Error( - "Landed on /change-password -- set SKIP_MUST_CHANGE_PASSWORD=true on the container for automated QA runs", + "Landed on /change-password; set SKIP_MUST_CHANGE_PASSWORD=true on the container for automated QA runs", ); } diff --git a/tests/unit/ai/bridge.test.ts b/tests/unit/ai/bridge.test.ts index a3031623..f5fe5692 100644 --- a/tests/unit/ai/bridge.test.ts +++ b/tests/unit/ai/bridge.test.ts @@ -1251,7 +1251,7 @@ describe("bridge - initDispatcher", () => { // pipe that teardown can surface as an EPIPE/ERR_STREAM_DESTROYED on the // stdin stream (observed in the container shutdown log as a spurious // "[bridge] Dispatcher crash #1"). The "close" handler alone is guarded, - // but the stdin error handler must be too -- otherwise shutdownDispatcher() + // but the stdin error handler must be too; otherwise shutdownDispatcher() // on every AI bundle install accrues false crashes toward the disable cap. const mock = createMockProcess(); vi.mocked(spawn).mockReturnValue(mock.process); diff --git a/tests/unit/image-engine/auto-enhance.test.ts b/tests/unit/image-engine/auto-enhance.test.ts index 7709819d..8722858b 100644 --- a/tests/unit/image-engine/auto-enhance.test.ts +++ b/tests/unit/image-engine/auto-enhance.test.ts @@ -366,7 +366,7 @@ describe("applyCorrections pipeline (CLAHE + normalise + gamma)", () => { const claheDisabledBuf = await claheDisabled.toBuffer(); // Skipping CLAHE via the size cap must produce byte-identical output to - // skipping it via the explicit toggle -- proof the cap actually took effect. + // skipping it via the explicit toggle: proof the cap actually took effect. expect(Buffer.compare(overCapBuf, claheDisabledBuf)).toBe(0); }); @@ -380,7 +380,7 @@ describe("applyCorrections pipeline (CLAHE + normalise + gamma)", () => { denoise: 0, }; // Use the real buffer's actual 200x150 dimensions (30,000 px, well under - // the 16M cap) -- CLAHE's tile size is derived from imageSize, and Sharp + // the 16M cap). CLAHE's tile size is derived from imageSize, and Sharp // rejects a tile window larger than the real underlying image, so a fake // imageSize far bigger than the actual small test buffer isn't valid here // (that's exactly what the "above the cap" test above uses instead, @@ -408,7 +408,7 @@ describe("applyCorrections pipeline (CLAHE + normalise + gamma)", () => { ); const claheDisabledBuf = await claheDisabled.toBuffer(); - // Under the cap, CLAHE should still run -- output must differ from the + // Under the cap, CLAHE should still run: output must differ from the // contrast-disabled baseline. expect(Buffer.compare(underCapBuf, claheDisabledBuf)).not.toBe(0); }); diff --git a/tests/unit/web/features-store-expanded.test.ts b/tests/unit/web/features-store-expanded.test.ts index b805da0b..801e1a37 100644 --- a/tests/unit/web/features-store-expanded.test.ts +++ b/tests/unit/web/features-store-expanded.test.ts @@ -202,7 +202,7 @@ describe("useFeaturesStore (expanded)", () => { await useFeaturesStore.getState().installBundle("done-bundle"); - // The client always POSTs now -- the server owns the queue/dedup decision. + // The client always POSTs now; the server owns the queue/dedup decision. expect(apiPostMock).toHaveBeenCalledWith("/v1/admin/features/done-bundle/install", {}); // A 409 already-installed clears installing + error and refreshes silently. const state = useFeaturesStore.getState(); diff --git a/tests/unit/web/features-store.test.ts b/tests/unit/web/features-store.test.ts index 0a4cc2f7..5001e96a 100644 --- a/tests/unit/web/features-store.test.ts +++ b/tests/unit/web/features-store.test.ts @@ -448,6 +448,17 @@ describe("useFeaturesStore", () => { }), ); + // The poll for the queued bundle reads server state from GET /v1/features. + let bundleBStatus: FeatureBundleState["status"] = "queued"; + apiGetMock.mockImplementation(() => + Promise.resolve({ + bundles: [ + makeBundleState({ id: "bundle-a", status: "installing" }), + makeBundleState({ id: "bundle-b", status: bundleBStatus }), + ], + }), + ); + await useFeaturesStore.getState().installAll(); await vi.waitFor(() => { @@ -456,21 +467,32 @@ describe("useFeaturesStore", () => { }); expect(useFeaturesStore.getState().installing["bundle-b"]).toBeUndefined(); - // Finish both; the second transitions queued -> installing on its first - // progress frame, then completes. - const esB = FakeEventSource.instances.find((es) => es.url.includes("bundle-b")); - esB?.onmessage?.({ data: JSON.stringify({ phase: "installing", percent: 10, stage: "Go" }) }); - await vi.waitFor(() => { - expect(useFeaturesStore.getState().queued).not.toContain("bundle-b"); - }); + // A queued bundle must NOT hold an SSE connection open (it polls + // instead): only the actively-installing bundle has an EventSource. + expect(FakeEventSource.instances).toHaveLength(1); + expect(FakeEventSource.instances[0].url).toContain("bundle-a"); - for (const es of FakeEventSource.instances) { - es.onmessage?.({ data: JSON.stringify({ phase: "complete" }) }); - } - await vi.waitFor(() => { - expect(useFeaturesStore.getState().installAllActive).toBe(false); - }); - }, 15000); + // The server starts bundle-b; its poll moves it queued -> installing. + bundleBStatus = "installing"; + await vi.waitFor( + () => { + expect(useFeaturesStore.getState().queued).not.toContain("bundle-b"); + expect(useFeaturesStore.getState().installing["bundle-b"]).toBeDefined(); + }, + { timeout: 8000 }, + ); + + // Finish both: bundle-a via its SSE stream, bundle-b via its poll + // observing the terminal server status. + bundleBStatus = "installed"; + FakeEventSource.instances[0].onmessage?.({ data: JSON.stringify({ phase: "complete" }) }); + await vi.waitFor( + () => { + expect(useFeaturesStore.getState().installAllActive).toBe(false); + }, + { timeout: 8000 }, + ); + }, 25000); it("retries a bundle once if it fails during Install All, then stops", async () => { const bundles = [makeBundleState({ id: "flaky", status: "not_installed" })];