mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
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
293 lines
11 KiB
TypeScript
293 lines
11 KiB
TypeScript
/**
|
|
* Integration tests for the server-side feature-install queue at the HTTP route
|
|
* level.
|
|
*
|
|
* 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:
|
|
* 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.
|
|
*/
|
|
import { randomUUID } from "node:crypto";
|
|
import { mkdirSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import { join } from "node:path";
|
|
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
// ── Hoisted mocks (spawn + venv lock) ────────────────────────────
|
|
const hoisted = vi.hoisted(() => {
|
|
// Minimal event emitter (no node:events import; vi.hoisted runs pre-import).
|
|
function makeEmitter() {
|
|
const listeners: Record<string, Array<(...a: unknown[]) => void>> = {};
|
|
return {
|
|
on(event: string, cb: (...a: unknown[]) => void) {
|
|
listeners[event] ??= [];
|
|
listeners[event].push(cb);
|
|
return this;
|
|
},
|
|
emit(event: string, ...args: unknown[]) {
|
|
for (const cb of listeners[event] ?? []) cb(...args);
|
|
},
|
|
};
|
|
}
|
|
|
|
interface FakeChild {
|
|
bundleId: string;
|
|
stdout: ReturnType<typeof makeEmitter>;
|
|
stderr: ReturnType<typeof makeEmitter>;
|
|
on: (event: string, cb: (...a: unknown[]) => void) => unknown;
|
|
emit: (event: string, ...args: unknown[]) => void;
|
|
}
|
|
|
|
const spawnCalls: FakeChild[] = [];
|
|
const spawnMock = vi.fn((_cmd: string, args: string[]) => {
|
|
const base = makeEmitter() as unknown as FakeChild;
|
|
base.bundleId = args[1];
|
|
base.stdout = makeEmitter();
|
|
base.stderr = makeEmitter();
|
|
spawnCalls.push(base);
|
|
return base;
|
|
});
|
|
|
|
const acquireVenvLockMock = vi.fn(async () => () => {});
|
|
const shutdownDispatcherMock = vi.fn();
|
|
|
|
return { spawnCalls, spawnMock, acquireVenvLockMock, shutdownDispatcherMock };
|
|
});
|
|
|
|
vi.mock("node:child_process", async (importOriginal) => {
|
|
const actual = (await importOriginal()) as typeof import("node:child_process");
|
|
return { ...actual, spawn: hoisted.spawnMock };
|
|
});
|
|
|
|
vi.mock("@snapotter/ai", async (importOriginal) => {
|
|
const actual = (await importOriginal()) as Record<string, unknown>;
|
|
return {
|
|
...actual,
|
|
acquireVenvLock: hoisted.acquireVenvLockMock,
|
|
shutdownDispatcher: hoisted.shutdownDispatcherMock,
|
|
};
|
|
});
|
|
|
|
// ── Temp DATA_DIR before importing feature-status ────────────────
|
|
const testRoot = join(tmpdir(), `snapotter-install-queue-${randomUUID()}`);
|
|
const aiDir = join(testRoot, "ai");
|
|
const modelsDir = join(aiDir, "models");
|
|
const installedPath = join(aiDir, "installed.json");
|
|
|
|
process.env.DATA_DIR = testRoot;
|
|
// Point at the real manifest so isDockerEnvironment() is true (GET /features
|
|
// then goes through getFeatureStates instead of the native "all installed"
|
|
// short-circuit) and import bundleId validation has a manifest to read.
|
|
process.env.FEATURE_MANIFEST_PATH = join(process.cwd(), "docker/feature-manifest.json");
|
|
|
|
mkdirSync(modelsDir, { recursive: true });
|
|
writeFileSync(installedPath, JSON.stringify({ bundles: {} }), "utf-8");
|
|
|
|
// ── Dynamic imports (after env + mocks) ──────────────────────────
|
|
const { acquireInstallLock, releaseInstallLock, invalidateCache } = await import(
|
|
"../../../apps/api/src/lib/feature-status.js"
|
|
);
|
|
const queue = await import("../../../apps/api/src/lib/feature-install-queue.js");
|
|
const { createMultipartPayload, loginAsAdmin } = await import("../test-server.js");
|
|
|
|
// ── Helpers ──────────────────────────────────────────────────────
|
|
|
|
async function waitFor(cond: () => boolean, timeoutMs = 3000): Promise<void> {
|
|
const start = Date.now();
|
|
while (!cond()) {
|
|
if (Date.now() - start > timeoutMs) throw new Error("waitFor: condition not met in time");
|
|
await new Promise((r) => setTimeout(r, 5));
|
|
}
|
|
}
|
|
|
|
async function tick(): Promise<void> {
|
|
await new Promise((r) => setTimeout(r, 40));
|
|
}
|
|
|
|
describe("POST /api/v1/admin/features/:bundleId/install queue", () => {
|
|
let app: Awaited<ReturnType<typeof import("fastify")>>["default"] extends (
|
|
...args: infer _A
|
|
) => infer R
|
|
? R
|
|
: never;
|
|
let token: string;
|
|
|
|
beforeAll(async () => {
|
|
const Fastify = (await import("fastify")).default;
|
|
const multipartPlugin = (await import("@fastify/multipart")).default;
|
|
const cookie = (await import("@fastify/cookie")).default;
|
|
const cors = (await import("@fastify/cors")).default;
|
|
|
|
app = Fastify({ logger: false, bodyLimit: 100 * 1024 * 1024 });
|
|
|
|
await app.register(cors, { origin: true });
|
|
await app.register(multipartPlugin, { limits: { fileSize: 100 * 1024 * 1024 } });
|
|
await app.register(cookie, { secret: "test-cookie-secret", hook: "onRequest" });
|
|
|
|
const { authMiddleware, authRoutes, ensureBuiltinRoles, ensureDefaultAdmin } = await import(
|
|
"../../../apps/api/src/plugins/auth.js"
|
|
);
|
|
await authMiddleware(app);
|
|
await authRoutes(app);
|
|
await ensureBuiltinRoles();
|
|
await ensureDefaultAdmin();
|
|
|
|
const { db, schema } = await import("../../../apps/api/src/db/index.js");
|
|
const { eq } = await import("drizzle-orm");
|
|
await db
|
|
.update(schema.users)
|
|
.set({ mustChangePassword: false })
|
|
.where(eq(schema.users.username, "admin"));
|
|
|
|
const { registerFeatureRoutes } = await import("../../../apps/api/src/routes/features.js");
|
|
await registerFeatureRoutes(app);
|
|
|
|
token = await loginAsAdmin(app);
|
|
});
|
|
|
|
afterAll(async () => {
|
|
if (app) await app.close();
|
|
});
|
|
|
|
beforeEach(() => {
|
|
queue.resetQueueState();
|
|
try {
|
|
releaseInstallLock();
|
|
} catch {
|
|
// no lock held
|
|
}
|
|
writeFileSync(installedPath, JSON.stringify({ bundles: {} }), "utf-8");
|
|
invalidateCache();
|
|
hoisted.spawnCalls.length = 0;
|
|
hoisted.spawnMock.mockClear();
|
|
});
|
|
|
|
const auth = () => ({ authorization: `Bearer ${token}` });
|
|
|
|
async function postInstall(bundleId: string) {
|
|
return app.inject({
|
|
method: "POST",
|
|
url: `/api/v1/admin/features/${bundleId}/install`,
|
|
headers: auth(),
|
|
});
|
|
}
|
|
|
|
async function getFeatures() {
|
|
const res = await app.inject({ method: "GET", url: "/api/v1/features", headers: auth() });
|
|
return JSON.parse(res.body).bundles as Array<{ id: string; status: string }>;
|
|
}
|
|
|
|
it("first install starts immediately (queued: false) and spawns once", async () => {
|
|
const res = await postInstall("ocr");
|
|
expect(res.statusCode).toBe(202);
|
|
const body = JSON.parse(res.body);
|
|
expect(body.queued).toBe(false);
|
|
expect(typeof body.jobId).toBe("string");
|
|
|
|
await waitFor(() => hoisted.spawnCalls.length === 1);
|
|
expect(hoisted.spawnCalls[0].bundleId).toBe("ocr");
|
|
});
|
|
|
|
it("a concurrent install is queued (202 queued: true) and does NOT spawn a second process", async () => {
|
|
const r1 = await postInstall("ocr");
|
|
expect(JSON.parse(r1.body).queued).toBe(false);
|
|
await waitFor(() => hoisted.spawnCalls.length === 1);
|
|
|
|
const r2 = await postInstall("face-detection");
|
|
expect(r2.statusCode).toBe(202);
|
|
expect(JSON.parse(r2.body).queued).toBe(true);
|
|
|
|
// Give any (incorrect) spawn a chance to fire; it must not.
|
|
await tick();
|
|
expect(hoisted.spawnCalls.length).toBe(1);
|
|
|
|
const bundles = await getFeatures();
|
|
expect(bundles.find((b) => b.id === "ocr")?.status).toBe("installing");
|
|
expect(bundles.find((b) => b.id === "face-detection")?.status).toBe("queued");
|
|
});
|
|
|
|
it("dedups a duplicate install POST of the active bundle (no second entry, same job)", async () => {
|
|
const r1 = await postInstall("ocr");
|
|
const jobId1 = JSON.parse(r1.body).jobId;
|
|
await waitFor(() => hoisted.spawnCalls.length === 1);
|
|
|
|
// POST the SAME bundle again while it is active.
|
|
const r2 = await postInstall("ocr");
|
|
expect(r2.statusCode).toBe(202);
|
|
const body2 = JSON.parse(r2.body);
|
|
expect(body2.queued).toBe(false);
|
|
// Returns the in-flight job id, not a new one.
|
|
expect(body2.jobId).toBe(jobId1);
|
|
|
|
await tick();
|
|
expect(hoisted.spawnCalls.length).toBe(1);
|
|
});
|
|
|
|
it("auto-starts the next queued bundle when the running install finishes", async () => {
|
|
await postInstall("ocr");
|
|
await waitFor(() => hoisted.spawnCalls.length === 1);
|
|
const r2 = await postInstall("face-detection");
|
|
expect(JSON.parse(r2.body).queued).toBe(true);
|
|
|
|
// The running install (ocr) finishes successfully.
|
|
hoisted.spawnCalls[0].emit("close", 0);
|
|
|
|
// The queued face-detection install auto-starts.
|
|
await waitFor(() => hoisted.spawnCalls.length === 2);
|
|
expect(hoisted.spawnCalls[1].bundleId).toBe("face-detection");
|
|
|
|
const bundles = await getFeatures();
|
|
expect(bundles.find((b) => b.id === "face-detection")?.status).toBe("installing");
|
|
|
|
// Cleanup: let the second install finish too.
|
|
hoisted.spawnCalls[1].emit("close", 0);
|
|
await waitFor(() => queue.getActiveBundleId() === null);
|
|
});
|
|
|
|
it("a bundle queued while an import holds the lock starts after the import route releases it", async () => {
|
|
// Simulate an offline import in progress by holding the install lock.
|
|
expect(acquireInstallLock("__import__")).toBe(true);
|
|
|
|
const res = await postInstall("ocr");
|
|
expect(res.statusCode).toBe(202);
|
|
expect(JSON.parse(res.body).queued).toBe(true);
|
|
|
|
// pump() could not acquire the held lock, so nothing spawned.
|
|
await tick();
|
|
expect(hoisted.spawnCalls.length).toBe(0);
|
|
const queuedBundles = await getFeatures();
|
|
expect(queuedBundles.find((b) => b.id === "ocr")?.status).toBe("queued");
|
|
|
|
// The import finishes and releases the lock; a subsequent import request's
|
|
// `finally { pump() }` then picks up the still-queued bundle.
|
|
releaseInstallLock();
|
|
|
|
const { body, contentType } = createMultipartPayload([
|
|
{
|
|
name: "file",
|
|
filename: "bad.tar.gz",
|
|
contentType: "application/gzip",
|
|
content: Buffer.from("not a real tarball"),
|
|
},
|
|
]);
|
|
const importRes = await app.inject({
|
|
method: "POST",
|
|
url: "/api/v1/admin/features/import",
|
|
headers: { ...auth(), "content-type": contentType },
|
|
payload: body,
|
|
});
|
|
expect(importRes.statusCode).toBeGreaterThanOrEqual(400);
|
|
|
|
// The import route's finally pumped the queue -> ocr now installs.
|
|
await waitFor(() => hoisted.spawnCalls.length === 1);
|
|
expect(hoisted.spawnCalls[0].bundleId).toBe("ocr");
|
|
|
|
// Cleanup.
|
|
hoisted.spawnCalls[0].emit("close", 0);
|
|
await waitFor(() => queue.getActiveBundleId() === null);
|
|
});
|
|
});
|