mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix(passport-photo): require the face-detection bundle, not just background-removal (#329)
* fix(passport-photo): require the face-detection bundle, not just background-removal Passport Photo runs face-landmark detection (face_landmarks.py, gated to the face-detection bundle) before background removal (background-removal bundle), but it was only declared under and guarded against background-removal. A user who installed only Background Removal passed every JS-side check, then hit a late "feature_not_installed" from the Python dispatcher gate when the analyze step ran face landmarks, and the UI never told them Face Detection was needed. - shared: add TOOL_EXTRA_BUNDLES + getRequiredBundlesForTool so a tool can declare more than one required bundle (passport-photo needs background-removal and face-detection). enablesTools is untouched, so the one-tool-per-bundle invariant still holds. - api: isToolInstalled() now checks every required bundle; add getFirstMissingBundleForTool() so the analyze and base routes, pipeline (both guards) and batch report the bundle the user actually still needs. - web: the proactive install prompt (tool-page) and features-store treat a tool as installed only when all required bundles are present, and point the prompt at the first missing one (sequential install, no new UI). Refs #327 * test(passport-photo): deterministic integration coverage for the two-bundle guard Boots the real API with an isolated DATA_DIR and controls installed.json to prove the HTTP route behavior end-to-end: - nothing installed -> 501 naming background-removal - only background-removal installed -> 501 naming face-detection (issue #327) - both installed -> guard passes (not 501) - base route reports face-detection too Refs #327
This commit is contained in:
@@ -18,7 +18,7 @@ import { dirname, join, resolve } from "node:path";
|
||||
import type { Readable } from "node:stream";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import type { FeatureBundleState, FeatureStatus } from "@snapotter/shared";
|
||||
import { FEATURE_BUNDLES, TOOL_BUNDLE_MAP } from "@snapotter/shared";
|
||||
import { FEATURE_BUNDLES, getRequiredBundlesForTool } from "@snapotter/shared";
|
||||
import * as tar from "tar";
|
||||
|
||||
// ── Paths ───────────────────────────────────────────────────────────────
|
||||
@@ -146,9 +146,22 @@ export function isFeatureInstalled(bundleId: string): boolean {
|
||||
}
|
||||
|
||||
export function isToolInstalled(toolId: string): boolean {
|
||||
const bundleId = TOOL_BUNDLE_MAP[toolId];
|
||||
if (!bundleId) return true;
|
||||
return isFeatureInstalled(bundleId);
|
||||
const required = getRequiredBundlesForTool(toolId);
|
||||
if (required.length === 0) return true;
|
||||
return required.every((bundleId) => isFeatureInstalled(bundleId));
|
||||
}
|
||||
|
||||
/**
|
||||
* The first required bundle for a tool that is not yet installed, or null when
|
||||
* the tool needs no bundle or all of them are installed. A tool can require
|
||||
* more than one bundle (see TOOL_EXTRA_BUNDLES), so this is what tells the user
|
||||
* exactly which feature to install next.
|
||||
*/
|
||||
export function getFirstMissingBundleForTool(toolId: string): string | null {
|
||||
for (const bundleId of getRequiredBundlesForTool(toolId)) {
|
||||
if (!isFeatureInstalled(bundleId)) return bundleId;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// ── Install status mutations ────────────────────────────────────────────
|
||||
|
||||
@@ -12,7 +12,7 @@ import { randomUUID } from "node:crypto";
|
||||
import { mkdir } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { getBundleForTool, TOOL_BUNDLE_MAP, TOOLS, toolSection } from "@snapotter/shared";
|
||||
import { FEATURE_BUNDLES, TOOLS, toolSection } from "@snapotter/shared";
|
||||
import archiver from "archiver";
|
||||
import type { FlowJob } from "bullmq";
|
||||
import { eq } from "drizzle-orm";
|
||||
@@ -26,7 +26,7 @@ import { type Pool, queueName, type ToolJobData } from "../jobs/types.js";
|
||||
import { autoOrient } from "../lib/auto-orient.js";
|
||||
import { getSecurityHeaders } from "../lib/csp.js";
|
||||
import { formatZodErrors } from "../lib/errors.js";
|
||||
import { isToolInstalled } from "../lib/feature-status.js";
|
||||
import { getFirstMissingBundleForTool } from "../lib/feature-status.js";
|
||||
import { validateImageBuffer } from "../lib/file-validation.js";
|
||||
import { sanitizeFilename } from "../lib/filename.js";
|
||||
import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js";
|
||||
@@ -79,13 +79,14 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
|
||||
return reply.status(404).send({ error: `Tool "${toolId}" not found` });
|
||||
}
|
||||
|
||||
// Guard: check if the tool's AI feature bundle is installed
|
||||
if (!isToolInstalled(toolId)) {
|
||||
const bundle = getBundleForTool(toolId);
|
||||
// Guard: check that every AI feature bundle the tool needs is installed
|
||||
const missingBundleId = getFirstMissingBundleForTool(toolId);
|
||||
if (missingBundleId) {
|
||||
const bundle = FEATURE_BUNDLES[missingBundleId];
|
||||
return reply.status(501).send({
|
||||
error: "Feature not installed",
|
||||
code: "FEATURE_NOT_INSTALLED",
|
||||
feature: TOOL_BUNDLE_MAP[toolId],
|
||||
feature: missingBundleId,
|
||||
featureName: bundle?.name ?? toolId,
|
||||
estimatedSize: bundle?.estimatedSize ?? "unknown",
|
||||
});
|
||||
|
||||
@@ -11,7 +11,7 @@ import { randomUUID } from "node:crypto";
|
||||
import { mkdir } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { ANALYTICS_EVENTS, getBundleForTool, TOOL_BUNDLE_MAP, TOOLS } from "@snapotter/shared";
|
||||
import { ANALYTICS_EVENTS, FEATURE_BUNDLES, TOOLS } from "@snapotter/shared";
|
||||
import archiver from "archiver";
|
||||
import type { FlowJob } from "bullmq";
|
||||
import { eq } from "drizzle-orm";
|
||||
@@ -26,7 +26,7 @@ import { trackEvent } from "../lib/analytics.js";
|
||||
import { autoOrient } from "../lib/auto-orient.js";
|
||||
import { getSecurityHeaders } from "../lib/csp.js";
|
||||
import { formatZodErrors } from "../lib/errors.js";
|
||||
import { isToolInstalled } from "../lib/feature-status.js";
|
||||
import { getFirstMissingBundleForTool } from "../lib/feature-status.js";
|
||||
import { validateImageBuffer } from "../lib/file-validation.js";
|
||||
import { sanitizeFilename } from "../lib/filename.js";
|
||||
import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js";
|
||||
@@ -361,12 +361,13 @@ export async function registerPipelineRoutes(app: FastifyInstance): Promise<void
|
||||
}
|
||||
|
||||
// Guard: check if the tool's AI feature bundle is installed
|
||||
if (!isToolInstalled(resolvedToolId)) {
|
||||
const bundle = getBundleForTool(resolvedToolId);
|
||||
const missingBundleId = getFirstMissingBundleForTool(resolvedToolId);
|
||||
if (missingBundleId) {
|
||||
const bundle = FEATURE_BUNDLES[missingBundleId];
|
||||
return reply.status(501).send({
|
||||
error: `Step ${i + 1} (${step.toolId}): Feature "${bundle?.name}" is not installed`,
|
||||
code: "FEATURE_NOT_INSTALLED",
|
||||
feature: TOOL_BUNDLE_MAP[resolvedToolId],
|
||||
feature: missingBundleId,
|
||||
featureName: bundle?.name ?? resolvedToolId,
|
||||
});
|
||||
}
|
||||
@@ -787,12 +788,13 @@ export async function registerPipelineRoutes(app: FastifyInstance): Promise<void
|
||||
});
|
||||
}
|
||||
|
||||
if (!isToolInstalled(resolvedToolId)) {
|
||||
const bundle = getBundleForTool(resolvedToolId);
|
||||
const missingBundleId = getFirstMissingBundleForTool(resolvedToolId);
|
||||
if (missingBundleId) {
|
||||
const bundle = FEATURE_BUNDLES[missingBundleId];
|
||||
return reply.status(501).send({
|
||||
error: `Step ${i + 1} (${step.toolId}): Feature "${bundle?.name}" is not installed`,
|
||||
code: "FEATURE_NOT_INSTALLED",
|
||||
feature: TOOL_BUNDLE_MAP[resolvedToolId],
|
||||
feature: missingBundleId,
|
||||
featureName: bundle?.name ?? resolvedToolId,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -3,18 +3,13 @@ import { mkdir, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { detectFaceLandmarks, removeBackground } from "@snapotter/ai";
|
||||
import {
|
||||
getBundleForTool,
|
||||
PASSPORT_SPECS,
|
||||
PRINT_LAYOUTS,
|
||||
TOOL_BUNDLE_MAP,
|
||||
} from "@snapotter/shared";
|
||||
import { FEATURE_BUNDLES, PASSPORT_SPECS, PRINT_LAYOUTS } from "@snapotter/shared";
|
||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||
import sharp from "sharp";
|
||||
import { z } from "zod";
|
||||
import { autoOrient } from "../../lib/auto-orient.js";
|
||||
import { formatZodErrors } from "../../lib/errors.js";
|
||||
import { isToolInstalled } from "../../lib/feature-status.js";
|
||||
import { getFirstMissingBundleForTool } from "../../lib/feature-status.js";
|
||||
import { validateImageBuffer } from "../../lib/file-validation.js";
|
||||
import { sanitizeFilename } from "../../lib/filename.js";
|
||||
import { decodeToSharpCompat, needsCliDecode } from "../../lib/format-decoders.js";
|
||||
@@ -134,12 +129,15 @@ export function registerPassportPhoto(app: FastifyInstance) {
|
||||
"/api/v1/tools/image/passport-photo/analyze",
|
||||
async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
const toolId = "passport-photo";
|
||||
if (!isToolInstalled(toolId)) {
|
||||
const bundle = getBundleForTool(toolId);
|
||||
// Passport Photo needs two bundles (face-detection + background-removal);
|
||||
// report whichever one the user still has to install.
|
||||
const missingBundleId = getFirstMissingBundleForTool(toolId);
|
||||
if (missingBundleId) {
|
||||
const bundle = FEATURE_BUNDLES[missingBundleId];
|
||||
return reply.status(501).send({
|
||||
error: "Feature not installed",
|
||||
code: "FEATURE_NOT_INSTALLED",
|
||||
feature: TOOL_BUNDLE_MAP[toolId],
|
||||
feature: missingBundleId,
|
||||
featureName: bundle?.name ?? toolId,
|
||||
estimatedSize: bundle?.estimatedSize ?? "unknown",
|
||||
});
|
||||
@@ -313,12 +311,13 @@ export function registerPassportPhoto(app: FastifyInstance) {
|
||||
"/api/v1/tools/image/passport-photo",
|
||||
async (_request: FastifyRequest, reply: FastifyReply) => {
|
||||
const toolId = "passport-photo";
|
||||
if (!isToolInstalled(toolId)) {
|
||||
const bundle = getBundleForTool(toolId);
|
||||
const missingBundleId = getFirstMissingBundleForTool(toolId);
|
||||
if (missingBundleId) {
|
||||
const bundle = FEATURE_BUNDLES[missingBundleId];
|
||||
return reply.status(501).send({
|
||||
error: "Feature not installed",
|
||||
code: "FEATURE_NOT_INSTALLED",
|
||||
feature: TOOL_BUNDLE_MAP[toolId],
|
||||
feature: missingBundleId,
|
||||
featureName: bundle?.name ?? toolId,
|
||||
estimatedSize: bundle?.estimatedSize ?? "unknown",
|
||||
});
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import {
|
||||
getRequiredBundlesForTool,
|
||||
PYTHON_SIDECAR_TOOLS,
|
||||
SECTIONS,
|
||||
TOOL_BUNDLE_MAP,
|
||||
TOOLS,
|
||||
toolSection,
|
||||
} from "@snapotter/shared";
|
||||
@@ -226,9 +226,17 @@ export function ToolPage() {
|
||||
const fetchFeatures = useFeaturesStore((s) => s.fetch);
|
||||
const featureBundle = useMemo(() => {
|
||||
if (!toolId) return null;
|
||||
const bundleId = TOOL_BUNDLE_MAP[toolId];
|
||||
if (!bundleId) return null;
|
||||
return featureBundles.find((b) => b.id === bundleId) ?? null;
|
||||
const required = getRequiredBundlesForTool(toolId);
|
||||
if (required.length === 0) return null;
|
||||
// A tool can need more than one bundle (e.g. passport-photo needs
|
||||
// background-removal AND face-detection). Surface the first one that is
|
||||
// still missing so the install prompt asks for what's actually needed;
|
||||
// once everything is installed, fall back to the primary bundle.
|
||||
for (const bundleId of required) {
|
||||
const bundle = featureBundles.find((b) => b.id === bundleId);
|
||||
if (bundle && bundle.status !== "installed") return bundle;
|
||||
}
|
||||
return featureBundles.find((b) => b.id === required[0]) ?? null;
|
||||
}, [toolId, featureBundles]);
|
||||
const toolInstalled = featureBundle ? featureBundle.status === "installed" : !isAiTool;
|
||||
const showSizeComparison = toolId === "compress" || toolId === "optimize-for-web";
|
||||
|
||||
@@ -1,8 +1,20 @@
|
||||
import type { FeatureBundleState } from "@snapotter/shared";
|
||||
import { TOOL_BUNDLE_MAP } from "@snapotter/shared";
|
||||
import { TOOL_BUNDLE_MAP, TOOL_EXTRA_BUNDLES } from "@snapotter/shared";
|
||||
import { create } from "zustand";
|
||||
import { apiGet, apiPost } from "@/lib/api";
|
||||
|
||||
/**
|
||||
* Every bundle a tool needs: its primary bundle plus any extras. Computed from
|
||||
* the exported maps (not the shared helper) so unit tests that mock
|
||||
* TOOL_BUNDLE_MAP still drive this logic. A tool can need more than one bundle
|
||||
* (e.g. passport-photo needs background-removal AND face-detection).
|
||||
*/
|
||||
function requiredBundlesForTool(toolId: string): string[] {
|
||||
const primary = TOOL_BUNDLE_MAP[toolId];
|
||||
if (!primary) return [];
|
||||
return [...new Set([primary, ...(TOOL_EXTRA_BUNDLES[toolId] ?? [])])];
|
||||
}
|
||||
|
||||
interface BundleProgress {
|
||||
percent: number;
|
||||
stage: string;
|
||||
@@ -197,16 +209,24 @@ export const useFeaturesStore = create<FeaturesState>((set, get) => {
|
||||
refresh: refreshBundles,
|
||||
|
||||
isToolInstalled: (toolId: string) => {
|
||||
const bundleId = TOOL_BUNDLE_MAP[toolId];
|
||||
if (!bundleId) return true;
|
||||
const bundle = get().bundles.find((b) => b.id === bundleId);
|
||||
return bundle?.status === "installed";
|
||||
const required = requiredBundlesForTool(toolId);
|
||||
if (required.length === 0) return true;
|
||||
return required.every(
|
||||
(bundleId) => get().bundles.find((b) => b.id === bundleId)?.status === "installed",
|
||||
);
|
||||
},
|
||||
|
||||
getBundleForTool: (toolId: string) => {
|
||||
const bundleId = TOOL_BUNDLE_MAP[toolId];
|
||||
if (!bundleId) return null;
|
||||
return get().bundles.find((b) => b.id === bundleId) ?? null;
|
||||
const required = requiredBundlesForTool(toolId);
|
||||
if (required.length === 0) return null;
|
||||
const bundles = get().bundles;
|
||||
// Point the user at the first bundle they still need to install; fall
|
||||
// back to the primary bundle once everything required is installed.
|
||||
for (const bundleId of required) {
|
||||
const bundle = bundles.find((b) => b.id === bundleId);
|
||||
if (bundle && bundle.status !== "installed") return bundle;
|
||||
}
|
||||
return bundles.find((b) => b.id === required[0]) ?? null;
|
||||
},
|
||||
|
||||
installBundle: async (bundleId: string) => {
|
||||
|
||||
Reference in New Issue
Block a user