From 669398821971f2253599af1e29902cc9d1d74fa6 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 4 Sep 2026 06:37:49 +0000 Subject: [PATCH 001/194] fix(isolation): route remaining raw fs writers into data/ through guarded primitives (#6203) - Add rmGuarded, unlinkGuarded, and createWriteStreamGuarded primitives to fileCore - Route ~40 raw fs calls (writes, unlinks, removes, streams) across imageGen, videoGen, sprites, writersRoom, pipeline, loras, agents, etc. through guarded primitives - Update testDataIsolation header and server/lib/README.md to document full coverage - Update affected tests to mock or assert on guarded primitives --- server/lib/README.md | 4 +- server/lib/fileCore.js | 22 +++++++++- server/lib/testDataIsolation.js | 12 ++--- server/lib/testDataIsolation.test.js | 28 +++++++++++- server/routes/attachments.js | 5 ++- server/routes/brainSongbook.js | 4 +- server/routes/imageGen.js | 7 ++- server/routes/imageVideoModels.js | 7 ++- server/routes/loraTraining.js | 5 +-- server/routes/uploads.js | 7 +-- server/routes/videoGen.test.js | 13 ++++-- server/services/agentCliSpawning.js | 14 +++--- server/services/agentCliSpawning.test.js | 3 +- server/services/agentLifecycle.js | 8 ++-- server/services/askConversations.js | 8 ++-- .../services/autonomousJobs/scriptHandlers.js | 6 +-- .../services/autonomousJobs/skillTemplates.js | 5 +-- server/services/catalogIngestSources.js | 5 +-- server/services/catalogIngestSources.test.js | 8 ++-- server/services/catalogMedia.js | 5 +-- server/services/chatgptZipImport.js | 44 +++++++++++++------ server/services/chiptune.js | 12 ++--- server/services/clinvar.js | 18 ++++---- server/services/clinvar.test.js | 15 ++++++- server/services/dataManager.js | 16 +++---- server/services/dataManager.test.js | 21 ++++----- server/services/datadog.js | 10 +---- server/services/datadog.test.js | 11 +++-- server/services/featureAgents.js | 10 ++--- server/services/genome.js | 9 ++-- server/services/googleAuth.js | 3 +- server/services/imageCleanTmpGc.js | 6 +-- server/services/imageGen/agy.js | 15 +++---- server/services/imageGen/cleanGpu.js | 9 ++-- server/services/imageGen/codex.js | 8 ++-- server/services/imageGen/external.js | 5 +-- server/services/imageGen/external.test.js | 2 +- server/services/imageGen/frameGuard.js | 5 +-- server/services/imageGen/grok.js | 18 +++----- server/services/imageGen/local.js | 22 +++++----- server/services/imageGen/prepareParams.js | 11 +++-- server/services/imageGen/variants.js | 4 +- server/services/imageRefsGc.js | 6 +-- .../imageTo3d/models.genericDispatch.test.js | 2 + server/services/imageTo3d/models.js | 10 ++--- server/services/imageTo3d/models.test.js | 6 +-- server/services/jira.js | 9 +--- server/services/jira.test.js | 12 ++++- server/services/loraDatasetGenerate.js | 6 +-- server/services/loraDatasets.js | 11 +++-- server/services/loraTraining/index.js | 13 +++--- server/services/loras.js | 22 +++++----- server/services/mediaSketches.js | 10 ++--- server/services/mediaSketches.test.js | 1 + server/services/memory.test.js | 10 ++--- server/services/memoryStore.js | 5 +-- server/services/missions.js | 8 ++-- server/services/moodBoard/pinterest.js | 5 +-- server/services/obsidian.js | 10 ++--- ...rsistentMindSupervisor.attachments.test.js | 2 + server/services/persistentMindSupervisor.js | 12 ++--- server/services/pipeline/audio.js | 5 +-- server/services/pipeline/musicLibrary.js | 12 ++--- server/services/sharing/exporter.js | 8 ++-- server/services/shellImageDrop.js | 5 +-- server/services/shellImageDrop.test.js | 1 + .../sprites/animationTrackWorkflow.js | 6 +-- server/services/sprites/assets.js | 5 +-- server/services/sprites/importer.js | 12 ++--- .../services/sprites/localAnimationRender.js | 5 +-- server/services/sprites/normalize.js | 7 ++- server/services/sprites/reference.js | 6 +-- server/services/sprites/walk.js | 6 +-- server/services/sprites/walkTrims.js | 10 ++--- server/services/subAgentSpawner.js | 6 +-- server/services/supersededBackup.js | 10 ++--- server/services/syncOrchestrator.js | 6 +-- server/services/taste-questionnaire.js | 6 +-- server/services/universeCharacterSheet.js | 7 ++- .../universeCharacterSheetDelete.test.js | 2 + server/services/videoGen/chainedVideo.js | 5 +-- .../videoGen/deleteHistoryItem.test.js | 1 + server/services/videoGen/frameExtraction.js | 19 ++++---- server/services/videoGen/generateVideo.js | 8 ++-- server/services/videoGen/grok.js | 16 +++---- server/services/videoGen/historyOps.js | 9 ++-- server/services/videoGen/local.test.js | 28 +++++++----- server/services/videoGen/prepareParams.js | 21 +++++---- .../services/videoGen/prepareParams.test.js | 11 ++++- server/services/videoGen/spawnWatch.js | 8 ++-- server/services/videoGen/stitchVideos.js | 7 ++- server/services/videoGen/upscaleVideo.js | 7 ++- server/services/videoUpload.js | 7 ++- server/services/voice/profiles.js | 5 ++- server/services/writersRoom/evaluator.js | 6 +-- server/services/writersRoom/local.js | 6 +-- server/services/writersRoom/sync.js | 5 +-- 97 files changed, 487 insertions(+), 417 deletions(-) diff --git a/server/lib/README.md b/server/lib/README.md index e541fa9cd8..6f2c2cbdf2 100644 --- a/server/lib/README.md +++ b/server/lib/README.md @@ -233,7 +233,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub | `dataRoot.js` | Data-root resolution + worktree-checkout detection (#1947). `resolveInstallRoot(fallbackRoot)` prefers the `PORTOS_DATA_ROOT` env var (pinned at real launch in `ecosystem.config.cjs`) over an `import.meta.url`-derived fallback, so a process booted from inside a CoS agent git worktree still resolves `data/`/`data.reference/` to the real install instead of the worktree's empty tree. `isWorktreeRoot(rootDir)` is the boot-migration backstop — true when `rootDir` lives under `data/cos/worktrees/` (keyed on the path segment only, so a fresh install's empty `data/` isn't a false positive). `resolveCodeRootForModule(moduleUrl)` is the single source of truth for the "two directories above this file" depth assumption — `paths.js`'s `CODE_ROOT` and `services/userActions.js`'s data-root guard both derive through it so they cannot silently drift apart. `DATA_ROOT_ENV` is the env-var name constant. Consumed by `fileUtils.js` (`PATHS`), `server/index.js`, and `scripts/run-migrations.js`. | | `downloadPreflight.js` | Free-disk preflight + resumable weight download. `assessDownloadPreflight({ destPath, expectedBytes })` → `{ freeBytes, requiredBytes, headroomBytes, verdict: ok\|tight\|insufficient }`; `assertDownloadFits` throws typed `DISK_INSUFFICIENT` (507) when the volume cannot hold the payload. `streamResumableDownload` Range-resumes a `.partial` on transport failure, discards it on user cancel, and verifies a published sha256 before rename. `sweepOrphanedPartials(dirs, { maxAgeMs })` unlinks leftover `.partial` (+ `.partial.etag`) files older than 7 days (default) and skips recent/in-flight dests; missing dirs are a no-op. `probeRemoteSize` / `siblingDownloadMeta` / `verifyDownloadHash` are the size+digest helpers the four weight-download entry points share. `createDownloadSlot({ codePrefix, idleStallEnvVar, exclusive, keepPartialOnCancel })` is the one transfer-slot registry a weight download claims before its first await — in-flight map, abort controller carrying a cancel/stall reason, typed `_DOWNLOAD_IN_FLIGHT`/`_STALLED`/`_CANCELLED` errors, a per-chunk progress throttle, and an `isInFlight(path)` predicate covering the dest, its `.partial`, and shards under a claimed directory. Call it at module scope only. `isAnyDownloadInFlight` answers for every registered slot at once, which is how the orphaned-partial GC protects live transfers. | | `agentInstructionsFile.js` | The `AGENTS.md` + bridge `CLAUDE.md` pair a repo carries (#4852). `writeAgentInstructions(repoPath, content)` writes the body to `AGENTS.md` and the one-line `@AGENTS.md` import beside it — use it in scaffolders instead of a bare `writeFile(join(repoPath, 'CLAUDE.md'), …)`, since a generated repo carrying only one name is unreadable to half the CLIs PortOS can point at it. Constants: `AGENT_INSTRUCTIONS_FILENAME`, `CLAUDE_BRIDGE_FILENAME`, `AGENT_INSTRUCTIONS_IMPORT`. | -| `fileCore.js` | Cross-cutting filesystem primitives (`atomicWrite`, directory helpers, bounded tail reads/watchers), time/format helpers, directory sizing, and SHA-256 helpers. | +| `fileCore.js` | Cross-cutting filesystem primitives (`atomicWrite`, `writeFileGuarded`, `appendFileGuarded`, `copyFileGuarded`, `rmGuarded`, `unlinkGuarded`, `createWriteStreamGuarded`, directory helpers, bounded tail reads/watchers), time/format helpers, directory sizing, and SHA-256 helpers. | | `fileUtils.js` | Backward-compatible facade re-exporting the focused file utility modules so existing deep imports need no caller changes. | | `portosEnv.js` | Single server-side helper for PortOS's own `.env` (`PORTOS_ENV_PATH` anchored to `installRoot` per #1947, `parseEnvContents`, `readPortosEnvValue`, `upsertPortosEnvLine` + `upsertEnvLine`), with replacer-function guard for `$`-patterns and `process.env` wins precedence; `scripts/lib/envFile.js` stays as the zero-dependency boundary copy. | | `secretText.js` | `scrubSecretTokens(text)` — replace credential-SHAPED substrings (prefixed API keys, GitHub/Slack tokens, JWTs, AWS key ids, pasted Bearer headers, 48+-char hex) with `[REDACTED]` in free text bound for an LLM provider or a world-readable artifact; `scrubSecretTokensDeep(value)` walks arrays/plain objects and scrubs every string value. Value-side counterpart to the operator-action ledger's key-based `redactPayload` and `commandSecurity.js#redactOutput`'s JSON patterns; conservative so prose, 40-hex git SHAs, and short ids survive. | @@ -532,5 +532,5 @@ pm` default, `NPM_CONFIG_PREFIX`, nvm/Volta) installed `codex` successfully and | `mockPathsDataRoot.js` | Shared Vitest helpers for `PATHS.data → temp dir` and no-peer record creation guards. | | `settingsTestUtil.js` | `bindSettingsFile(dataRoot)` → `writeSettingsFile`/`mergeSettingsFile`: direct settings.json disk writes that also drop the `getSettings()` read cache (dynamic-import reset) so a stale cache can't survive a bypass-`save()` write. | | `runtimeEnv.js` | `isTestRunner()` — NODE_ENV=test **or** the VITEST env var, so a run that dropped NODE_ENV is still armed. Dependency-free and deliberately apart from `db.js`, which used to own it: the lowest-level file primitives need the same answer and must not pull in `pg`, and the many suites spelling `vi.mock('../lib/db.js', () => ({ query }))` used to strip it out of the graph for every other consumer. | -| `testDataIsolation.js` | Runtime backstop against a test WRITING into the install's real `data/` tree — the filesystem analogue of `db.js`'s row-write guard. `isInsideRealDataRoot(path)` canonicalizes a target through symlinked ancestors (it need not exist yet) and tests containment with `pathSafety.js#isPathAtOrInsideDir`, so a `..` climb, a relative path, a `data-archive` sibling, or a differently-cased spelling on a case-insensitive filesystem can't walk in sideways; the real root is re-derived through `dataRoot.js` rather than read from `PATHS`, so a suite that redirects `PATHS.data` to a temp root can't also redirect the guard. `assertNotRealDataWrite(path, operation)` throws under the test runner from `atomicWrite`, the `writeFile`/`appendFile`/`copyFile` wrappers in `fileCore.js`, and `collectionStore`'s record delete; `assertNotNewRealDataDir(dir)` is the create-only variant `ensureDir` uses, since `mkdir -p` on an existing directory mutates nothing. Inert outside the runner — the `isTestRunner()` check precedes every syscall. NOT yet universal: ~40 services still write to `PATHS.*` with raw `fs`, tracked as follow-up. Closes the write half of the bug class `testDataIsolation.guards.test.js` covers statically and the two-run probe covers for reads (#6176, after #6171). | +| `testDataIsolation.js` | Runtime backstop against a test WRITING into the install's real `data/` tree — the filesystem analogue of `db.js`'s row-write guard. `isInsideRealDataRoot(path)` canonicalizes a target through symlinked ancestors (it need not exist yet) and tests containment with `pathSafety.js#isPathAtOrInsideDir`, so a `..` climb, a relative path, a `data-archive` sibling, or a differently-cased spelling on a case-insensitive filesystem can't walk in sideways; the real root is re-derived through `dataRoot.js` rather than read from `PATHS`, so a suite that redirects `PATHS.data` to a temp root can't also redirect the guard. `assertNotRealDataWrite(path, operation)` throws under the test runner from `atomicWrite`, the guarded wrappers in `fileCore.js` (`writeFileGuarded`, `appendFileGuarded`, `copyFileGuarded`, `rmGuarded`, `unlinkGuarded`, `createWriteStreamGuarded`), and `collectionStore`'s record delete; `assertNotNewRealDataDir(dir)` is the create-only variant `ensureDir` uses, since `mkdir -p` on an existing directory mutates nothing. Inert outside the runner — the `isTestRunner()` check precedes every syscall. Services mutating files under `data/` route through these guarded wrappers. Closes the write half of the bug class `testDataIsolation.guards.test.js` covers statically and the two-run probe covers for reads (#6176, after #6171, #6203). | | `testHelper.js` | Test helpers: `request()` (supertest-style HTTP) + `mockJsonResponse`/`mockTextResponse` (fetch `Response` mocks with `.text()`, `.json()`, and a `headers.get` content-type), `startLoopbackServer(app)`/`closeLoopbackServer(server)`/`waitForAbort(signal)` for tests that need a real socket (raw disconnects, SSE streaming) that `request()`'s run-to-completion fetch harness can't model, plus the source-scan pair `collectServerSources()` / `readServerSource(rel)` (and `SERVER_DIR`) used by the whole-tree guard suites — `spawnCwd.test.js` (#3193) and `cliChildEnv.test.js` (#3194). Those guards overlap deliberately, so they share one definition of "a source file"; change the ignore rules here and both move together. Cross-platform trio: `posixPath(v)` normalizes a RECEIVED path before comparing it to a POSIX-spelled literal (no-op on POSIX — never normalize the expectation, which would hide a genuinely wrong path), and `resolveTestPython()` returns an interpreter that actually runs, probing by execution because Windows ships a `python` Store-alias stub that exists but fails; `null` when there is none, for `describe.skipIf`; `pinPlatform(value)` pins `process.platform` and returns a restore that reinstates the ORIGINAL descriptor (deleting the pin when there was none) — it carries the one hazard every hand-rolled pin had to rediscover: never pin above an import that loads a native addon, which picks its prebuilt binary off the platform at load time (#4085). Python-shelling suites also take their two nested budgets from here: `PY_TEST_TIMEOUT_MS` (vitest per-test, passed as `it()`'s third argument — a real interpreter's wall time tracks machine load, not the assertion, so a ~4s case crosses the tight global 10s `testTimeout` on a contended full-suite worker) and the strictly smaller `PY_SUBPROCESS_TIMEOUT_MS` (every `execFileSync` spawn's own `timeout`, so a hung interpreter trips the spawn guard first and names the command instead of producing a bare vitest timeout; a subprocess allowance ABOVE the vitest budget is dead intent — vitest always wins). | diff --git a/server/lib/fileCore.js b/server/lib/fileCore.js index bc4639b59a..6a53b121d6 100644 --- a/server/lib/fileCore.js +++ b/server/lib/fileCore.js @@ -1,6 +1,6 @@ /** Cross-cutting filesystem, time, formatting, and hashing helpers. */ -import { access, appendFile, chmod, copyFile, mkdir, readFile, readdir, stat, writeFile, rename, unlink } from 'fs/promises'; -import { createReadStream, existsSync, statSync, watch as watchFileSystem } from 'fs'; +import { access, appendFile, chmod, copyFile, mkdir, readFile, readdir, stat, writeFile, rename, unlink, rm } from 'fs/promises'; +import { createReadStream, createWriteStream, existsSync, statSync, watch as watchFileSystem } from 'fs'; import { createHash, randomUUID } from 'crypto'; import { basename, dirname, extname, join } from 'path'; import { promisify } from 'util'; @@ -279,6 +279,24 @@ export async function copyFileGuarded(src, dest, mode) { return copyFile(src, dest, mode); } +/** Guarded `fs/promises.rm`. Same signature. */ +export async function rmGuarded(target, options) { + if (isVitestRunner()) (await loadGuard()).assertNotRealDataWrite(target, 'rm'); + return rm(target, options); +} + +/** Guarded `fs/promises.unlink`. Same signature. */ +export async function unlinkGuarded(target) { + if (isVitestRunner()) (await loadGuard()).assertNotRealDataWrite(target, 'unlink'); + return unlink(target); +} + +/** Guarded `fs.createWriteStream`. Returns a Promise resolving to the WriteStream. */ +export async function createWriteStreamGuarded(filePath, options) { + if (isVitestRunner()) (await loadGuard()).assertNotRealDataWrite(filePath, 'createWriteStream'); + return createWriteStream(filePath, options); +} + export const MINUTE = 60 * 1000; export const HOUR = 60 * MINUTE; export const DAY = 24 * HOUR; diff --git a/server/lib/testDataIsolation.js b/server/lib/testDataIsolation.js index ef21a92627..40981330ee 100644 --- a/server/lib/testDataIsolation.js +++ b/server/lib/testDataIsolation.js @@ -41,14 +41,10 @@ * * Under the runner it fires from the shared primitives in `fileCore.js` / * `jsonIo.js` (`atomicWrite`, `ensureDir`'s create path, `writeFileGuarded`, - * `appendFileGuarded`, `copyFileGuarded`, `appendJSONLine`) and from - * `collectionStore`'s record delete. It is NOT yet universal: roughly forty - * services still reach `PATHS.*` with raw `fs` calls of their own (see - * `sharing/importer.js`, `videoUpload.js`, `catalogMedia.js`, `genome.js`), - * and on a populated install their target directories already exist, so - * `ensureDir`'s create-path check is a no-op for them. Routing those onto the - * guarded wrappers is tracked as follow-up work — do not read this module as - * proof that every write is covered. + * `appendFileGuarded`, `copyFileGuarded`, `rmGuarded`, `unlinkGuarded`, + * `createWriteStreamGuarded`, `appendJSONLine`) and from + * `collectionStore`'s record delete. All services mutating files under `data/` + * route through these guarded wrappers (#6203). * * A suite that genuinely needs a data root redirects it with * `createTempDataRoot()` + `makePathsProxy()` from `lib/mockPathsDataRoot.js`; diff --git a/server/lib/testDataIsolation.test.js b/server/lib/testDataIsolation.test.js index bd8b3e30e4..e848a7e411 100644 --- a/server/lib/testDataIsolation.test.js +++ b/server/lib/testDataIsolation.test.js @@ -18,7 +18,7 @@ import { describe, it, expect, vi, afterEach, afterAll } from 'vitest'; import { existsSync, mkdtempSync, readFileSync, rmSync, symlinkSync } from 'fs'; import { tmpdir } from 'os'; import { isAbsolute, join, relative, sep } from 'path'; -import { appendFileGuarded, atomicWrite, copyFileGuarded, ensureDir, writeFileGuarded } from './fileCore.js'; +import { appendFileGuarded, atomicWrite, copyFileGuarded, createWriteStreamGuarded, ensureDir, rmGuarded, unlinkGuarded, writeFileGuarded } from './fileCore.js'; import { appendJSONLine } from './jsonIo.js'; import { assertNotNewRealDataDir, assertNotRealDataWrite, isInsideRealDataRoot } from './testDataIsolation.js'; import { isPathAtOrInsideDir } from './pathContainment.js'; @@ -175,6 +175,32 @@ describe('the guarded raw-fs wrappers', () => { await copyFileGuarded(src, join(tempRoot, 'dest.txt')); expect(readFileSync(join(tempRoot, 'dest.txt'), 'utf8')).toBe('payload'); }); + + it('rmGuarded refuses the real tree and allows a temp root', async () => { + await expect(rmGuarded(join(REAL_DATA, 'probe.txt'))).rejects.toThrow(/rm refused/); + const target = join(tempRoot, 'to-rm.txt'); + await writeFileGuarded(target, 'x'); + await rmGuarded(target); + expect(existsSync(target)).toBe(false); + }); + + it('unlinkGuarded refuses the real tree and allows a temp root', async () => { + await expect(unlinkGuarded(join(REAL_DATA, 'probe.txt'))).rejects.toThrow(/unlink refused/); + const target = join(tempRoot, 'to-unlink.txt'); + await writeFileGuarded(target, 'x'); + await unlinkGuarded(target); + expect(existsSync(target)).toBe(false); + }); + + it('createWriteStreamGuarded refuses the real tree and allows a temp root', async () => { + await expect(createWriteStreamGuarded(join(REAL_DATA, 'probe.txt'))).rejects.toThrow(/createWriteStream refused/); + const target = join(tempRoot, 'streamed.txt'); + const ws = await createWriteStreamGuarded(target); + await new Promise((res, rej) => { + ws.write('streamed content', (err) => (err ? rej(err) : ws.end(res))); + }); + expect(readFileSync(target, 'utf8')).toBe('streamed content'); + }); }); describe('appendJSONLine', () => { diff --git a/server/routes/attachments.js b/server/routes/attachments.js index b6d7acfe89..0a446ef3e3 100644 --- a/server/routes/attachments.js +++ b/server/routes/attachments.js @@ -4,12 +4,13 @@ */ import { Router } from 'express'; -import { unlink, readdir, stat } from 'fs/promises'; +import { readdir, stat } from 'fs/promises'; import { join, resolve } from 'path'; import { asyncHandler, ServerError } from '../lib/errorHandler.js'; import { pathExists, PATHS, sanitizeFilename, getFileExtension, getMimeType, ATTACHMENT_ALLOWED_EXTENSIONS, isPathInsideDir, saveBase64Upload, serveLocalFile, + unlinkGuarded, } from '../lib/fileUtils.js'; import { MAX_BASE64_UPLOAD_BYTES } from '../lib/uploadLimits.js'; import { validateRequest, attachmentUploadRequestSchema } from '../lib/validation.js'; @@ -65,7 +66,7 @@ router.delete('/:filename', asyncHandler(async (req, res) => { throw new ServerError('Attachment not found', { status: 404, code: 'NOT_FOUND' }); } - await unlink(filepath); + await unlinkGuarded(filepath); console.log(`🗑️ Attachment deleted: ${safeFilename}`); diff --git a/server/routes/brainSongbook.js b/server/routes/brainSongbook.js index ef46696ccb..c12d086d2d 100644 --- a/server/routes/brainSongbook.js +++ b/server/routes/brainSongbook.js @@ -19,7 +19,6 @@ */ import { Router } from 'express'; -import { unlink } from 'fs/promises'; import { join, resolve } from 'path'; import { createHash } from 'crypto'; import { asyncHandler, ServerError } from '../lib/errorHandler.js'; @@ -38,6 +37,7 @@ import { MAX_BASE64_UPLOAD_BYTES } from '../lib/uploadLimits.js'; import { pathExists, PATHS, sanitizeFilename, isPathInsideDir, SONGBOOK_ATTACHMENT_EXTENSIONS, saveBase64Upload, serveLocalFile, + unlinkGuarded, } from '../lib/fileUtils.js'; const router = Router(); @@ -231,7 +231,7 @@ router.delete('/:id/attachments/:filename', asyncHandler(async (req, res) => { // Bytes may legitimately be absent on this machine (meta synced from a peer). if (await pathExists(filepath)) { - await unlink(filepath); + await unlinkGuarded(filepath); } console.log(`🗑️ Song attachment deleted: ${safeFilename}`); diff --git a/server/routes/imageGen.js b/server/routes/imageGen.js index eaffd75e8b..1177847ef0 100644 --- a/server/routes/imageGen.js +++ b/server/routes/imageGen.js @@ -11,7 +11,6 @@ import { Router } from 'express'; import { z } from 'zod'; -import { unlink, copyFile } from 'fs/promises'; import { asyncHandler, ServerError, failValidation } from '../lib/errorHandler.js'; import { validateRequest, imageEdgeSchema, refineImagePixelCap, PIXEL_CAP_MESSAGE, @@ -26,7 +25,7 @@ import { recordUserAction } from '../services/userActions.js'; import { getImageModels, requiredReposForModel } from '../lib/mediaModels.js'; import { inspectModelCache, verifyModelCache, repairModelCache, aggregateVerifies } from '../lib/hfCache.js'; import { startHfDownloadStream } from '../services/hfDownloadStream.js'; -import { PATHS, ensureDir, resolveGalleryImage } from '../lib/fileUtils.js'; +import { PATHS, ensureDir, resolveGalleryImage, unlinkGuarded, copyFileGuarded } from '../lib/fileUtils.js'; import { prepareGenerateParams, resolveLocalImageModel, selectLocalImageModel } from '../services/imageGen/prepareParams.js'; import { applyImageClean, applyWatermarkRemoval, applyLightRegenVariant } from '../services/imageGen/variants.js'; import { join, basename } from 'node:path'; @@ -535,7 +534,7 @@ router.post('/generate', imageGenUploads, asyncHandler(async (req, res) => { // the client drops the connection mid-flight. if (uploadedTempPaths.length) { res.on('close', () => { - for (const p of uploadedTempPaths) unlink(p).catch(() => {}); + for (const p of uploadedTempPaths) unlinkGuarded(p).catch(() => {}); }); } // Local + codex both go through mediaJobQueue (separate lanes — codex @@ -970,7 +969,7 @@ router.post('/:filename/regenerate', asyncHandler(async (req, res) => { if (annotatedSketchPath) { await ensureDir(PATHS.imageRefs); initImageAbsPath = join(PATHS.imageRefs, `init-${randomUUID()}.png`); - await copyFile(annotatedSketchPath, initImageAbsPath); + await copyFileGuarded(annotatedSketchPath, initImageAbsPath); } // Provider-aware default (issue #912): SynthID-bearing sources keep the diff --git a/server/routes/imageVideoModels.js b/server/routes/imageVideoModels.js index fadab87aaf..dd5c23a6b8 100644 --- a/server/routes/imageVideoModels.js +++ b/server/routes/imageVideoModels.js @@ -10,11 +10,10 @@ import { Router } from 'express'; import { existsSync } from 'fs'; -import { rm } from 'fs/promises'; import { join } from 'path'; import { z } from 'zod'; import { asyncHandler, ServerError } from '../lib/errorHandler.js'; -import { PATHS } from '../lib/fileUtils.js'; +import { PATHS, rmGuarded } from '../lib/fileUtils.js'; import { getHfCacheRoot } from '../lib/hfCache.js'; import { getImageModels, @@ -175,7 +174,7 @@ router.delete('/hf/:dirName', asyncHandler(async (req, res) => { const fullPath = join(HF_HUB_DIR(), dirName); if (!existsSync(fullPath)) throw new ServerError('Model not found', { status: 404, code: 'NOT_FOUND' }); console.log(`🗑️ Deleting HF model cache: ${dirName}`); - await rm(fullPath, { recursive: true, force: true }); + await rmGuarded(fullPath, { recursive: true, force: true }); res.json({ ok: true }); })); @@ -187,7 +186,7 @@ router.delete('/lora/:filename', asyncHandler(async (req, res) => { const filePath = join(PATHS.loras, filename); if (!existsSync(filePath)) throw new ServerError('LoRA not found', { status: 404, code: 'NOT_FOUND' }); console.log(`🗑️ Deleting LoRA: ${filename}`); - await rm(filePath, { force: true }); + await rmGuarded(filePath, { force: true }); res.json({ ok: true }); })); diff --git a/server/routes/loraTraining.js b/server/routes/loraTraining.js index 394f12d421..d2497e01fc 100644 --- a/server/routes/loraTraining.js +++ b/server/routes/loraTraining.js @@ -7,12 +7,11 @@ */ import { Router } from 'express'; -import { rm } from 'fs/promises'; import { join } from 'path'; import { z } from 'zod'; import { asyncHandler, sendErrorResponse, ServerError } from '../lib/errorHandler.js'; import { startTrainingRunSchema, validateRequest } from '../lib/validation.js'; -import { assertSafeFilename } from '../lib/fileUtils.js'; +import { assertSafeFilename, rmGuarded } from '../lib/fileUtils.js'; import { resolveFlux2Python, isFlux2VenvHealthy, resolveMfluxPython } from '../lib/pythonSetup.js'; import { getSettings } from '../services/settings.js'; import { attachSseClient, cancelJob } from '../services/mediaJobQueue/index.js'; @@ -115,7 +114,7 @@ router.delete('/runs/:id', asyncHandler(async (req, res) => { } // Artifact dir is server-derived from the run id (uuid) — confined under // PATHS.trainingRuns by construction. - await rm(runDir(run.id), { recursive: true, force: true }); + await rmGuarded(runDir(run.id), { recursive: true, force: true }); if (req.query.deleteLora === 'true' && run.output?.loraFilename) { await deleteLora(run.output.loraFilename).catch((err) => { console.log(`⚠️ trained LoRA delete skipped: ${err?.message}`); diff --git a/server/routes/uploads.js b/server/routes/uploads.js index a5f03bb5fe..8c887cd26e 100644 --- a/server/routes/uploads.js +++ b/server/routes/uploads.js @@ -4,12 +4,13 @@ */ import { Router } from 'express'; -import { unlink, readdir, stat } from 'fs/promises'; +import { readdir, stat } from 'fs/promises'; import { join, resolve } from 'path'; import { asyncHandler, ServerError } from '../lib/errorHandler.js'; import { pathExists, PATHS, sanitizeFilename, getFileExtension, getMimeType, EXTENSION_MIME_MAP, isPathInsideDir, saveBase64Upload, serveLocalFile, + unlinkGuarded, } from '../lib/fileUtils.js'; import { MAX_BASE64_UPLOAD_BYTES } from '../lib/uploadLimits.js'; import { validateRequest, uploadRequestSchema } from '../lib/validation.js'; @@ -130,7 +131,7 @@ router.delete('/:filename', asyncHandler(async (req, res) => { } const stats = await stat(filepath); - await unlink(filepath); + await unlinkGuarded(filepath); console.log(`🗑️ File deleted: ${safeFilename} (${formatSize(stats.size)})`); @@ -156,7 +157,7 @@ router.delete('/', asyncHandler(async (req, res) => { const filepath = join(UPLOADS_DIR, filename); const stats = await stat(filepath); freedSpace += stats.size; - await unlink(filepath); + await unlinkGuarded(filepath); } console.log(`🗑️ Cleared all uploads: ${files.length} files (${formatSize(freedSpace)})`); diff --git a/server/routes/videoGen.test.js b/server/routes/videoGen.test.js index d4dba66796..aa602080fd 100644 --- a/server/routes/videoGen.test.js +++ b/server/routes/videoGen.test.js @@ -238,8 +238,13 @@ vi.mock('../lib/multipart.js', () => ({ }, })); +const fsMock = vi.hoisted(() => ({ + unlink: vi.fn(async () => {}), + copyFile: vi.fn(async () => {}), +})); + vi.mock('../lib/fileUtils.js', () => ({ -tryReadFile: vi.fn().mockResolvedValue(null), + tryReadFile: vi.fn().mockResolvedValue(null), PATHS: { root: '/mock', data: '/mock/data', @@ -251,6 +256,8 @@ tryReadFile: vi.fn().mockResolvedValue(null), // Route awaits ensureDir before staging the upload; no-op for tests since // we mock copyFile too. ensureDir: vi.fn(async () => {}), + copyFileGuarded: fsMock.copyFile, + unlinkGuarded: fsMock.unlink, // The route resolves user-supplied basenames through this helper before // handing them to the renderer. Mirror the real helper's basename-strip // + dot-segment rejection so the "strips path-traversal" test below @@ -268,10 +275,10 @@ vi.mock('fs', () => ({ existsSync: vi.fn(() => true), })); vi.mock('fs/promises', () => ({ - unlink: vi.fn(async () => {}), + unlink: fsMock.unlink, // The route stages multipart uploads to data/uploads/ via copyFile. Stub // the copy so tests that simulate req.file don't actually touch disk. - copyFile: vi.fn(async () => {}), + copyFile: fsMock.copyFile, })); import { copyFile, unlink } from 'fs/promises'; diff --git a/server/services/agentCliSpawning.js b/server/services/agentCliSpawning.js index 0b80299e67..e4d6cfb2bf 100644 --- a/server/services/agentCliSpawning.js +++ b/server/services/agentCliSpawning.js @@ -6,7 +6,7 @@ */ import { join } from 'path'; -import { readFile, writeFile } from 'fs/promises'; +import { readFile } from 'fs/promises'; import { existsSync } from 'fs'; import { spawn } from '../lib/childProcess.js'; import { homedir } from 'os'; @@ -24,7 +24,7 @@ import { finalizeAgent, releaseAgentLane } from './agentFinalization.js'; import { activeAgents, userTerminatedAgents, pausedAgents, consumePausedAgentExit, registerSpawnedAgent, unregisterSpawnedAgent } from './agentState.js'; import { normalizeReviewers } from '../lib/validation.js'; import { resolveReviewLoopOptions } from './codeReview.js'; -import { safeJSONParse, PATHS } from '../lib/fileUtils.js'; +import { safeJSONParse, PATHS, writeFileGuarded } from '../lib/fileUtils.js'; import { createCodexStderrFormatter } from '../lib/codexCliOutput.js'; import { createStreamingAnsiStripper } from '../lib/ansiStrip.js'; import { PROVIDER_TYPES } from '../lib/aiToolkit/constants.js'; @@ -650,7 +650,7 @@ export async function spawnDirectly({ const lines = streamParser.processChunk(text); for (const line of lines) outputBuffer += line + '\n'; outputBatcher.push(lines); - await writeFile(outputFile, outputBuffer).catch(() => {}); + await writeFileGuarded(outputFile, outputBuffer).catch(() => {}); } else { // Non-stream providers: emit stdout as-is once decolored. A chunk that // was purely terminal control has nothing left to show. Unlike stderr @@ -658,7 +658,7 @@ export async function spawnDirectly({ // formatting when it isn't wearing an `[stderr]` tag. if (!text) return; outputBuffer += text; - await writeFile(outputFile, outputBuffer).catch(() => {}); + await writeFileGuarded(outputFile, outputBuffer).catch(() => {}); outputBatcher.push(text); } }); @@ -680,7 +680,7 @@ export async function spawnDirectly({ const lines = codexStderrFormatter.processChunk(text); for (const line of lines) outputBuffer += line + '\n'; outputBatcher.push(lines); - await writeFile(outputFile, outputBuffer).catch(() => {}); + await writeFileGuarded(outputFile, outputBuffer).catch(() => {}); return; } // A chunk that decolors down to whitespace was pure terminal control @@ -688,7 +688,7 @@ export async function spawnDirectly({ // `[stderr]` would add one blank noise line to the tail per redraw. if (!text.trim()) return; outputBuffer += `[stderr] ${text}`; - await writeFile(outputFile, outputBuffer).catch(() => {}); + await writeFileGuarded(outputFile, outputBuffer).catch(() => {}); outputBatcher.push(`[stderr] ${text}`); }); } catch (err) { @@ -814,7 +814,7 @@ export async function spawnDirectly({ // below too, since output.txt is written next). await outputBatcher.flush(); - await writeFile(outputFile, outputBuffer).catch(() => {}); + await writeFileGuarded(outputFile, outputBuffer).catch(() => {}); // Paused agents are finalized in `markAgentPaused` (which already released // the lane + execution). Return BEFORE `releaseAgentLane` below — re-running diff --git a/server/services/agentCliSpawning.test.js b/server/services/agentCliSpawning.test.js index b4da378c21..2875fbc1ef 100644 --- a/server/services/agentCliSpawning.test.js +++ b/server/services/agentCliSpawning.test.js @@ -68,11 +68,12 @@ vi.mock('./agentState.js', () => ({ metaStringOr: (value, fallback) => (typeof value === 'string' && value) ? value : fallback, })); vi.mock('../lib/fileUtils.js', () => ({ -tryReadFile: vi.fn().mockResolvedValue(null), + tryReadFile: vi.fn().mockResolvedValue(null), safeJSONParse: (str, fallback) => { try { return JSON.parse(str); } catch { return fallback; } }, // agentSentinel builds the per-agent sentinel filename with this — a mock // missing it makes doneSentinelPath throw inside the close handler. sanitizeFilename: (name) => String(name).replace(/[^a-zA-Z0-9._-]/g, '_'), + writeFileGuarded: vi.fn().mockResolvedValue(undefined), PATHS: { root: '/tmp', cosAgents: '/tmp/agents', data: '/tmp/data' }, })); vi.mock('../lib/codexCliOutput.js', () => ({ createCodexStderrFormatter: vi.fn() })); diff --git a/server/services/agentLifecycle.js b/server/services/agentLifecycle.js index 926a859d8f..789415fd61 100644 --- a/server/services/agentLifecycle.js +++ b/server/services/agentLifecycle.js @@ -26,7 +26,7 @@ */ import { join } from 'path'; -import { readFile, writeFile } from 'fs/promises'; +import { readFile } from 'fs/promises'; import { existsSync } from 'fs'; import { cosEvents, emitLog } from './cosEvents.js'; // The DEFINING module, not a barrel (#3450) — see the note in @@ -46,7 +46,7 @@ import { MAX_TOTAL_SPAWNS, normalizeReviewers } from '../lib/validation.js'; import { isInternalTaskId } from '../lib/taskParser.js'; import { isRetryHeld } from '../lib/taskRetryHold.js'; import { PROVIDER_CONFIG_BLOCKED_CATEGORY } from '../lib/taskBlockCategories.js'; -import { ensureDir, PATHS, sleep, tryReadFile } from '../lib/fileUtils.js'; +import { ensureDir, PATHS, sleep, tryReadFile, writeFileGuarded } from '../lib/fileUtils.js'; import { createToolExecution, startExecution, completeExecution, errorExecution } from './toolStateMachine.js'; import { determineLane, acquire, release } from './executionLanes.js'; import { analyzeAgentFailure } from './agentErrorAnalysis.js'; @@ -707,11 +707,11 @@ async function runAgentSpawn(task) { } // Save prompt to file - await writeFile(join(agentDir, 'prompt.txt'), prompt); + await writeFileGuarded(join(agentDir, 'prompt.txt'), prompt); let systemPromptFile = null; if (systemPrompt) { systemPromptFile = join(agentDir, 'system-prompt.md'); - await writeFile(systemPromptFile, systemPrompt); + await writeFileGuarded(systemPromptFile, systemPrompt); } // Create run entry for usage tracking diff --git a/server/services/askConversations.js b/server/services/askConversations.js index eb3631004d..ccfbca14bc 100644 --- a/server/services/askConversations.js +++ b/server/services/askConversations.js @@ -24,9 +24,9 @@ */ import { join } from 'path'; -import { readdir, stat, unlink } from 'fs/promises'; +import { readdir, stat } from 'fs/promises'; import { randomUUID } from 'crypto'; -import { PATHS, atomicWrite, ensureDir, readJSONFile, safeDate } from '../lib/fileUtils.js'; +import { PATHS, atomicWrite, ensureDir, readJSONFile, safeDate, unlinkGuarded } from '../lib/fileUtils.js'; export const ASK_DIR = join(PATHS.data, 'ask-conversations'); export const EXPIRY_DAYS = 30; @@ -138,7 +138,7 @@ export async function listConversations({ limit = 50 } = {}) { } if (!conv.promoted && effectiveTs && (now - effectiveTs) > expiryMs) { // Single-user app — no concurrency races to worry about pruning here. - await unlink(pathFor(id)).catch(() => {}); + await unlinkGuarded(pathFor(id)).catch(() => {}); continue; } @@ -222,7 +222,7 @@ export async function appendTurn(conversationId, turn) { export async function deleteConversation(id) { if (!isValidId(id)) return false; - const removed = await unlink(pathFor(id)).then(() => true, (err) => { + const removed = await unlinkGuarded(pathFor(id)).then(() => true, (err) => { if (err.code === 'ENOENT') return false; throw err; }); diff --git a/server/services/autonomousJobs/scriptHandlers.js b/server/services/autonomousJobs/scriptHandlers.js index 2c5f6b38c7..cf07c75686 100644 --- a/server/services/autonomousJobs/scriptHandlers.js +++ b/server/services/autonomousJobs/scriptHandlers.js @@ -6,11 +6,11 @@ * the job's `scriptHandler` name to its implementation. */ -import { readdir, stat, rm } from 'fs/promises' +import { readdir, stat } from 'fs/promises' import { join, dirname } from 'path' import { fileURLToPath } from 'url' import { existsSync } from 'fs' -import { spawn } from '../../lib/childProcess.js';import { DAY, PATHS, readJSONFile } from '../../lib/fileUtils.js' +import { spawn } from '../../lib/childProcess.js';import { DAY, PATHS, readJSONFile, rmGuarded } from '../../lib/fileUtils.js' import { checkAndPrompt as autobiographyCheckAndPrompt } from '../autobiography.js' import { runGoalCheckIn } from '../goalCheckIn.js' import { cleanupOrphanedWorktrees, reapMergedWorktrees } from '../worktreeManager.js' @@ -108,7 +108,7 @@ async function agentDataCleanup() { const info = await stat(entryPath).catch(() => null) if (!info?.isDirectory()) continue if (info.mtimeMs < cutoff) { - const removed = await rm(entryPath, { recursive: true, force: true }).then(() => true, (err) => { + const removed = await rmGuarded(entryPath, { recursive: true, force: true }).then(() => true, (err) => { console.warn(`⚠️ Failed to clean agent dir ${entry}: ${err.message}`) return false }) diff --git a/server/services/autonomousJobs/skillTemplates.js b/server/services/autonomousJobs/skillTemplates.js index 5e0f0fef53..8e9c1accae 100644 --- a/server/services/autonomousJobs/skillTemplates.js +++ b/server/services/autonomousJobs/skillTemplates.js @@ -6,9 +6,8 @@ * due job into a CoS task payload (`generateTaskFromJob`). */ -import { writeFile } from 'fs/promises' import { join } from 'path' -import { ensureDir, PATHS, tryReadFile } from '../../lib/fileUtils.js' +import { ensureDir, PATHS, tryReadFile, writeFileGuarded } from '../../lib/fileUtils.js' import { JOBS_SKILLS_DIR, JOB_SKILL_MAP } from './constants.js' import { getAppById } from '../apps.js' import { appendTaskDataInputs, resolveTaskDataInputs } from '../taskDataInputs.js' @@ -35,7 +34,7 @@ async function loadJobSkillTemplate(skillName) { async function saveJobSkillTemplate(skillName, content) { await ensureDir(JOBS_SKILLS_DIR) const filePath = join(JOBS_SKILLS_DIR, `${skillName}.md`) - await writeFile(filePath, content) + await writeFileGuarded(filePath, content) console.log(`💾 Saved job skill template: ${skillName}`) } diff --git a/server/services/catalogIngestSources.js b/server/services/catalogIngestSources.js index 38ba2e9c3a..c39991a252 100644 --- a/server/services/catalogIngestSources.js +++ b/server/services/catalogIngestSources.js @@ -24,7 +24,6 @@ */ import { randomUUID } from 'crypto'; -import { writeFile } from 'fs/promises'; import { join } from 'path'; import * as catalogDB from './catalogDB.js'; import * as brainStorage from './brainStorage.js'; @@ -32,7 +31,7 @@ import { extractIngredientsForScrap } from './catalogExtraction.js'; import { transcribe } from './voice/stt.js'; import { navigateToUrlPinned } from './browserService.js'; import { lookup } from 'dns/promises'; -import { PATHS, ensureDir, safeJSONParse } from '../lib/fileUtils.js'; +import { PATHS, ensureDir, safeJSONParse, writeFileGuarded } from '../lib/fileUtils.js'; import { isSafeIngestUrl, isBlockedIngestHost } from '../lib/catalogValidation.js'; // Cap fetched/transcribed bodies at the scrap column boundary (the Zod @@ -192,7 +191,7 @@ async function persistVoiceMemoAudio(audioBuffer, mimeType) { const ext = mimeType?.includes('webm') ? 'webm' : mimeType?.includes('mpeg') ? 'mp3' : 'wav'; const mediaKey = `voice-memo-${randomUUID()}.${ext}`; await ensureDir(PATHS.audio); - await writeFile(join(PATHS.audio, mediaKey), audioBuffer); + await writeFileGuarded(join(PATHS.audio, mediaKey), audioBuffer); return mediaKey; } diff --git a/server/services/catalogIngestSources.test.js b/server/services/catalogIngestSources.test.js index c235b1b665..acf0fbd4f2 100644 --- a/server/services/catalogIngestSources.test.js +++ b/server/services/catalogIngestSources.test.js @@ -45,6 +45,7 @@ vi.mock('../lib/fileUtils.js', () => ({ PATHS: { audio: '/tmp/data/audio' }, ensureDir: vi.fn(), sleep: vi.fn(() => Promise.resolve()), + writeFileGuarded: vi.fn().mockResolvedValue(undefined), safeJSONParse: vi.fn((raw, fallback) => { try { return JSON.parse(raw); @@ -60,6 +61,7 @@ const browserService = await import('./browserService.js'); const brainStorage = await import('./brainStorage.js'); const fsp = await import('fs/promises'); const dnsp = await import('dns/promises'); +const fileUtils = await import('../lib/fileUtils.js'); const { fetchUrlMainText, ingestFromUrl, @@ -254,10 +256,10 @@ describe('ingestFromVoice', () => { it('defaults the audio persister to write under PATHS.audio with a wav extension', async () => { const transcribeFn = vi.fn().mockResolvedValue({ text: 'hi' }); const { mediaKey } = await ingestFromVoice({ audioBase64: wavBase64 }, { transcribeFn }); - // Real persistFn ran: it called writeFile under the mocked PATHS.audio. - expect(fsp.writeFile).toHaveBeenCalledTimes(1); + // Real persistFn ran: it called writeFileGuarded under the mocked PATHS.audio. + expect(fileUtils.writeFileGuarded).toHaveBeenCalledTimes(1); expect(mediaKey).toMatch(/^voice-memo-.*\.wav$/); - const writtenPath = fsp.writeFile.mock.calls[0][0]; + const writtenPath = fileUtils.writeFileGuarded.mock.calls[0][0]; // Composed with path.join — accept either separator. expect(writtenPath.split('\\').join('/')).toContain('/tmp/data/audio'); }); diff --git a/server/services/catalogMedia.js b/server/services/catalogMedia.js index 75ef339dd6..71d8a51b3b 100644 --- a/server/services/catalogMedia.js +++ b/server/services/catalogMedia.js @@ -12,9 +12,8 @@ // replica hashes the file when it federates. import { randomUUID } from 'crypto'; -import { writeFile } from 'fs/promises'; import { join } from 'path'; -import { ensureDir, PATHS } from '../lib/fileUtils.js'; +import { ensureDir, PATHS, atomicWrite } from '../lib/fileUtils.js'; import { ServerError } from '../lib/errorHandler.js'; import { getIngredient, attachMedia } from './catalogDB.js'; import { saveUploadedGalleryImage } from './imageGen/local.js'; @@ -53,7 +52,7 @@ export function classifyUploadMime(mimeType) { async function persistLibraryFile(buffer, dir, ext) { const filename = `upload-${randomUUID().slice(0, 8)}.${ext}`; await ensureDir(dir); - await writeFile(join(dir, filename), buffer); + await atomicWrite(join(dir, filename), buffer); return filename; } diff --git a/server/services/chatgptZipImport.js b/server/services/chatgptZipImport.js index 75e3f134cc..5709eed95c 100644 --- a/server/services/chatgptZipImport.js +++ b/server/services/chatgptZipImport.js @@ -29,11 +29,11 @@ * only difference is the `assetResolver`. */ -import { createReadStream, createWriteStream } from 'fs'; -import { rename, unlink } from 'fs/promises'; +import { createReadStream } from 'fs'; +import { rename } from 'fs/promises'; import { join } from 'path'; import { Writable } from 'stream'; -import { PATHS, getMimeType, ensureDir } from '../lib/fileUtils.js'; +import { PATHS, getMimeType, ensureDir, createWriteStreamGuarded, unlinkGuarded } from '../lib/fileUtils.js'; import { isTopLevelEntryName } from '../lib/pathSafety.js'; import { parseZip, collectZipEntry, MAX_ZIP_MEMBER_BYTES } from '../lib/zipStream.js'; import { parseExport, importConversations, assetPointerId } from './chatgptImport.js'; @@ -153,15 +153,20 @@ const UNSAFE_MEMBER_WARNING = '⚠️ ChatGPT import: skipped a ZIP member with // Resolves with the sniffed extension (or null when the magic bytes aren't // recognized — the caller then falls back to the friendly name / `.bin`). const streamAssetToFile = (entry, filePath, max) => new Promise((resolve, reject) => { - const out = createWriteStream(filePath); + const outPromise = createWriteStreamGuarded(filePath); + let out = null; const head = []; let headLen = 0; let size = 0; let done = false; - const fail = (err) => { if (done) return; done = true; out.destroy(); reject(err); }; - out.on('error', fail); + const fail = (err) => { if (done) return; done = true; out?.destroy(); reject(err); }; + outPromise.then((stream) => { + out = stream; + out.on('error', fail); + out.on('finish', () => { if (done) return; done = true; resolve(sniffExtension(Buffer.concat(head))); }); + }).catch(fail); const sink = new Writable({ - write(chunk, _enc, cb) { + async write(chunk, _enc, cb) { size += chunk.length; if (size > max) { cb(new Error(`ZIP member exceeds ${max} byte limit`)); return; } if (headLen < SNIFF_BYTES) { @@ -171,13 +176,24 @@ const streamAssetToFile = (entry, filePath, max) => new Promise((resolve, reject head.push(slice); headLen += slice.length; } - if (out.write(chunk)) cb(); - else out.once('drain', cb); + try { + const stream = await outPromise; + if (stream.write(chunk)) cb(); + else stream.once('drain', cb); + } catch (err) { + cb(err); + } + }, + async final(cb) { + try { + const stream = await outPromise; + stream.end(cb); + } catch (err) { + cb(err); + } }, - final(cb) { out.end(cb); }, }); sink.on('error', fail); - out.on('finish', () => { if (done) return; done = true; resolve(sniffExtension(Buffer.concat(head))); }); entry.pipe(sink); }); @@ -309,7 +325,7 @@ export async function extractChatgptZip(zipPath, { assetDir = PATHS.brainImportA if (!isTopLevelEntryName(fileName)) { console.warn(UNSAFE_MEMBER_WARNING); // eslint-disable-next-line no-await-in-loop -- same sequential loop - await unlink(tempPath).catch(() => {}); + await unlinkGuarded(tempPath).catch(() => {}); continue; } const filePath = join(assetDir, fileName); @@ -379,7 +395,7 @@ export function makeAssetResolver(assets) { // served files behind, and repeated bad uploads could fill the disk. async function cleanupExtractedAssets(assets, assetDir = PATHS.brainImportAssets) { await Promise.all( - [...assets.values()].map((a) => unlink(`${assetDir}/${a.file}`).catch(() => {})) + [...assets.values()].map((a) => unlinkGuarded(`${assetDir}/${a.file}`).catch(() => {})) ); } @@ -388,7 +404,7 @@ async function cleanupExtractedAssets(assets, assetDir = PATHS.brainImportAssets // partial/aborted import doesn't leave orphaned temp files behind. Already- // renamed temps no longer exist — the unlink simply no-ops on them. async function cleanupTempFiles(tempPaths) { - await Promise.all([...tempPaths].map((p) => unlink(p).catch(() => {}))); + await Promise.all([...tempPaths].map((p) => unlinkGuarded(p).catch(() => {}))); } /** diff --git a/server/services/chiptune.js b/server/services/chiptune.js index 532a689fee..06d43f0b1e 100644 --- a/server/services/chiptune.js +++ b/server/services/chiptune.js @@ -14,11 +14,11 @@ * repo owns its own git — we only write files. */ -import { unlink, stat } from 'fs/promises'; -import { join, resolve, isAbsolute } from 'path'; +import { stat } from 'fs/promises'; import { randomUUID } from 'crypto'; +import { join, resolve, isAbsolute } from 'path'; import { ServerError } from '../lib/errorHandler.js'; -import { PATHS, atomicWrite, isPathInsideDir } from '../lib/fileUtils.js'; +import { PATHS, atomicWrite, isPathInsideDir, unlinkGuarded } from '../lib/fileUtils.js'; import { findFfmpeg, runFfmpegProcess } from '../lib/ffmpeg.js'; import { chiptuneScoreSchema, CHIPTUNE_LIMITS, CHIPTUNE_NOISE_PRESETS, scoreDurationSec } from '../lib/chiptuneScore.js'; import { renderScoreToWav } from '../lib/chiptuneRender.js'; @@ -128,16 +128,16 @@ async function renderScoreToFile(score, dir, basename) { // WAV fallback: drop any stale .ogg from an earlier ffmpeg-equipped // publish, or a game still referencing it would play the old composition // while the score JSON says otherwise. - await unlink(oggPath).catch(() => {}); + await unlinkGuarded(oggPath).catch(() => {}); return `${basename}.wav`; } const result = await runFfmpegProcess({ bin, args: ['-y', '-i', wavPath, '-c:a', 'libvorbis', '-q:a', '5', oggPath] }); if (!result.ok) { console.error(`❌ Chiptune OGG encode failed (keeping WAV): ${result.reason}`); - await unlink(oggPath).catch(() => {}); // stale or partial encode output + await unlinkGuarded(oggPath).catch(() => {}); // stale or partial encode output return `${basename}.wav`; } - await unlink(wavPath).catch(() => {}); + await unlinkGuarded(wavPath).catch(() => {}); return `${basename}.ogg`; } diff --git a/server/services/clinvar.js b/server/services/clinvar.js index b2ae416835..ee36504b93 100644 --- a/server/services/clinvar.js +++ b/server/services/clinvar.js @@ -1,10 +1,10 @@ -import { createWriteStream, createReadStream } from 'fs'; -import { writeFile, stat, unlink } from 'fs/promises'; +import { createReadStream } from 'fs'; +import { stat } from 'fs/promises'; import { join } from 'path'; import { createGunzip } from 'zlib'; import { createInterface } from 'readline'; import { pipeline } from 'stream/promises'; -import { atomicWrite, PATHS, ensureDir, safeJSONParse, tryReadFile } from '../lib/fileUtils.js'; +import { atomicWrite, PATHS, ensureDir, safeJSONParse, tryReadFile, createWriteStreamGuarded, unlinkGuarded } from '../lib/fileUtils.js'; const GENOME_DIR = PATHS.meatspace; const CLINVAR_GZ = join(GENOME_DIR, 'clinvar-raw.txt.gz'); @@ -65,7 +65,7 @@ async function downloadClinvar(onProgress) { } const totalBytes = parseInt(response.headers.get('content-length') || '0', 10); - const writer = createWriteStream(CLINVAR_GZ); + const writer = await createWriteStreamGuarded(CLINVAR_GZ); const reader = response.body.getReader(); let downloaded = 0; @@ -264,7 +264,7 @@ export async function syncClinvar(onProgress) { // Save compact index const indexJson = JSON.stringify(index); - await writeFile(CLINVAR_INDEX, indexJson); + await atomicWrite(CLINVAR_INDEX, indexJson); // Save metadata const meta = { @@ -277,7 +277,7 @@ export async function syncClinvar(onProgress) { await atomicWrite(CLINVAR_META, meta); // Clean up the raw gz file to save disk space - await unlink(CLINVAR_GZ).catch(() => {}); + await unlinkGuarded(CLINVAR_GZ).catch(() => {}); console.log(`🧬 ClinVar sync complete: ${meta.variantCount.toLocaleString()} variants indexed (${(meta.indexSize / 1024 / 1024).toFixed(1)}MB)`); @@ -370,9 +370,9 @@ export async function scanClinvar(snpIndex) { * Delete ClinVar data (index + meta + raw). */ export async function deleteClinvar() { - await unlink(CLINVAR_GZ).catch(() => {}); - await unlink(CLINVAR_INDEX).catch(() => {}); - await unlink(CLINVAR_META).catch(() => {}); + await unlinkGuarded(CLINVAR_GZ).catch(() => {}); + await unlinkGuarded(CLINVAR_INDEX).catch(() => {}); + await unlinkGuarded(CLINVAR_META).catch(() => {}); clinvarIndex = null; console.log('🧬 ClinVar data deleted'); return { success: true }; diff --git a/server/services/clinvar.test.js b/server/services/clinvar.test.js index 3624d2cfd0..9f7ba87d42 100644 --- a/server/services/clinvar.test.js +++ b/server/services/clinvar.test.js @@ -8,6 +8,15 @@ const NO_GENOME_ERROR = 'No genome data uploaded.'; // compact index + meta via tryReadFile and writes/deletes via fs/promises. const fileStore = new Map(); +const unlinkMock = vi.fn(async (path) => { + if (!fileStore.has(toPosix(path))) { + const err = new Error('ENOENT'); + err.code = 'ENOENT'; + throw err; + } + fileStore.delete(toPosix(path)); +}); + vi.mock('../lib/fileUtils.js', () => ({ PATHS: { meatspace: '/mock/meatspace' }, ensureDir: vi.fn().mockResolvedValue(undefined), @@ -18,7 +27,11 @@ vi.mock('../lib/fileUtils.js', () => ({ } catch { return fallback; } - } + }, + unlinkGuarded: unlinkMock, + atomicWrite: vi.fn(async (path, data) => { + fileStore.set(toPosix(path), typeof data === 'string' ? data : JSON.stringify(data)); + }), })); vi.mock('fs/promises', () => ({ diff --git a/server/services/dataManager.js b/server/services/dataManager.js index cd1c6e502a..5d19624b45 100644 --- a/server/services/dataManager.js +++ b/server/services/dataManager.js @@ -1,9 +1,9 @@ -import { readdir, stat, lstat, rm, writeFile as fsWriteFile } from 'fs/promises'; +import { readdir, stat, lstat } from 'fs/promises'; import { join, relative, resolve, isAbsolute } from 'path'; import { existsSync } from 'fs'; import { execFile } from '../lib/childProcess.js'; import { promisify } from 'util'; -import { PATHS, ensureDir, isTopLevelEntryName } from '../lib/fileUtils.js'; +import { PATHS, ensureDir, isTopLevelEntryName, rmGuarded, writeFileGuarded } from '../lib/fileUtils.js'; import { ServerError } from '../lib/errorHandler.js'; import { federatedMediaInboxBusy, imageCleanTmpBusy, trainingRunsBusy, updateDetachedBusy, @@ -322,12 +322,12 @@ export async function archiveCategory(categoryKey, options = {}) { // Write file list to temp file to avoid shell argument limits const listPath = join(backupDir, `.filelist-${Date.now()}.txt`); - await fsWriteFile(listPath, oldFiles.join('\n')); + await writeFileGuarded(listPath, oldFiles.join('\n')); await execFileAsync('tar', ['-czf', archivePath, '-C', dirPath, '-T', listPath], { timeout: 120000 }); - await rm(listPath).catch(() => {}); + await rmGuarded(listPath).catch(() => {}); for (const f of oldFiles) { - await rm(join(dirPath, f)).catch(() => {}); + await rmGuarded(join(dirPath, f)).catch(() => {}); } const archiveStat = await stat(archivePath).catch(() => null); @@ -418,11 +418,11 @@ export async function purgeCategory(categoryKey, options = {}) { ); } } - await rm(resolvedTarget, { recursive: !itemScoped, force: true }); + await rmGuarded(resolvedTarget, { recursive: !itemScoped, force: true }); console.log(`🗑️ Purged item from data/${categoryKey}`); } else { const entries = await readdir(dirPath).catch(() => []); - await Promise.all(entries.map(entry => rm(join(dirPath, entry), { recursive: true, force: true }))); + await Promise.all(entries.map(entry => rmGuarded(join(dirPath, entry), { recursive: true, force: true }))); console.log(`🗑️ Purged all ${entries.length} entries from data/${categoryKey}`); } @@ -467,6 +467,6 @@ export async function deleteBackup(filename) { if (!fullPath.startsWith(backupDir)) { throw new ServerError('Path traversal not allowed', { status: 400, code: 'VALIDATION_ERROR' }); } - await rm(fullPath); + await rmGuarded(fullPath); return { deleted: filename }; } diff --git a/server/services/dataManager.test.js b/server/services/dataManager.test.js index 13aca7e802..aa7986b771 100644 --- a/server/services/dataManager.test.js +++ b/server/services/dataManager.test.js @@ -10,43 +10,44 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -// Mock fs/promises so deleteBackup's rm is a no-op spy — the accept path must -// not touch the real filesystem, and reject paths throw before rm is reached. -vi.mock('fs/promises', async (importOriginal) => { +const { rmGuarded } = vi.hoisted(() => ({ + rmGuarded: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('../lib/fileUtils.js', async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, rm: vi.fn().mockResolvedValue(undefined) }; + return { ...actual, rmGuarded }; }); -import { rm } from 'fs/promises'; import { deleteBackup } from './dataManager.js'; describe('deleteBackup filename validation (#1822)', () => { beforeEach(() => { - rm.mockClear(); + rmGuarded.mockClear(); }); it('accepts a real dotted backup archive name', async () => { const result = await deleteBackup('agents-2026-06-30T12-34-56.tar.gz'); expect(result).toEqual({ deleted: 'agents-2026-06-30T12-34-56.tar.gz' }); - expect(rm).toHaveBeenCalledTimes(1); + expect(rmGuarded).toHaveBeenCalledTimes(1); }); it('rejects ".." traversal without touching the filesystem', async () => { await expect(deleteBackup('../secrets.json')).rejects.toThrow('Invalid filename'); await expect(deleteBackup('a/../../etc/passwd')).rejects.toThrow('Invalid filename'); - expect(rm).not.toHaveBeenCalled(); + expect(rmGuarded).not.toHaveBeenCalled(); }); it('rejects the bare "." and ".." entries (which resolve to the backup dir / its parent)', async () => { await expect(deleteBackup('.')).rejects.toThrow('Invalid filename'); await expect(deleteBackup('..')).rejects.toThrow('Invalid filename'); - expect(rm).not.toHaveBeenCalled(); + expect(rmGuarded).not.toHaveBeenCalled(); }); it('rejects path separators and other unsafe characters', async () => { await expect(deleteBackup('sub/dir.tar.gz')).rejects.toThrow('Invalid filename'); await expect(deleteBackup('name with spaces.tar.gz')).rejects.toThrow('Invalid filename'); await expect(deleteBackup('weird$name.tar.gz')).rejects.toThrow('Invalid filename'); - expect(rm).not.toHaveBeenCalled(); + expect(rmGuarded).not.toHaveBeenCalled(); }); }); diff --git a/server/services/datadog.js b/server/services/datadog.js index 55f7fd3707..0ef0f8e3f6 100644 --- a/server/services/datadog.js +++ b/server/services/datadog.js @@ -3,11 +3,10 @@ * Supports multiple DataDog instances with API/App key authentication */ -import fs from 'fs/promises'; import { createHttpClient } from '../lib/httpClient.js'; import { countConfiguredInstances } from '../lib/instanceFeatureRegistry.js'; import path from 'path'; -import { ensureDir, PATHS, readJSONFile } from '../lib/fileUtils.js'; +import { atomicWrite, ensureDir, PATHS, readJSONFile } from '../lib/fileUtils.js'; const DATADOG_CONFIG_FILE = path.join(PATHS.data, 'datadog.json'); @@ -48,12 +47,7 @@ export async function hasConfiguredInstances() { * Save DataDog instances configuration */ export async function saveInstances(config) { - await ensureDir(path.dirname(DATADOG_CONFIG_FILE)); - await fs.writeFile( - DATADOG_CONFIG_FILE, - JSON.stringify(config, null, 2), - 'utf-8' - ); + await atomicWrite(DATADOG_CONFIG_FILE, config); } /** diff --git a/server/services/datadog.test.js b/server/services/datadog.test.js index 0e06b562d2..c6b3ab708f 100644 --- a/server/services/datadog.test.js +++ b/server/services/datadog.test.js @@ -1,9 +1,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +const { writeFileMock } = vi.hoisted(() => ({ + writeFileMock: vi.fn(), +})); + vi.mock('fs/promises', () => { const mod = { readFile: vi.fn(), - writeFile: vi.fn(), + writeFile: writeFileMock, mkdir: vi.fn() }; return { ...mod, default: mod }; @@ -14,10 +18,11 @@ vi.mock('fs', () => ({ })); vi.mock('../lib/fileUtils.js', () => ({ -tryReadFile: vi.fn().mockResolvedValue(null), + tryReadFile: vi.fn().mockResolvedValue(null), PATHS: { data: '/mock/data' }, ensureDir: vi.fn(), - readJSONFile: vi.fn() + readJSONFile: vi.fn(), + atomicWrite: vi.fn(async (p, data) => writeFileMock(p, typeof data === 'string' ? data : JSON.stringify(data))), })); vi.mock('../lib/httpClient.js', () => ({ diff --git a/server/services/featureAgents.js b/server/services/featureAgents.js index 3e0560a956..c57f2c9516 100644 --- a/server/services/featureAgents.js +++ b/server/services/featureAgents.js @@ -6,12 +6,12 @@ * git worktree/branch and runs on a schedule. */ -import { writeFile, readFile, readdir, rm } from 'fs/promises'; +import { readFile, readdir } from 'fs/promises'; import { join } from 'path'; import { existsSync } from 'fs'; import { v4 as uuidv4 } from '../lib/uuid.js'; import { cosEvents } from './cosEvents.js'; -import { ensureDir, PATHS, readJSONFile, atomicWrite } from '../lib/fileUtils.js'; +import { ensureDir, PATHS, readJSONFile, atomicWrite, rmGuarded } from '../lib/fileUtils.js'; import { createMutex } from '../lib/asyncMutex.js'; import { isPlainObject } from '../lib/objects.js'; import { getAppById } from './apps.js'; @@ -160,7 +160,7 @@ export async function deleteFeatureAgent(id) { } // Remove the entire data directory (runs, etc.) if (existsSync(agentDir)) { - await rm(agentDir, { recursive: true, force: true }).catch(err => { + await rmGuarded(agentDir, { recursive: true, force: true }).catch(err => { console.log(`⚠️ Feature agent data cleanup failed: ${err.message}`); }); } @@ -432,9 +432,9 @@ export async function recordRunCompletion(id, runData) { const runDir = join(FA_DIR, id, 'runs'); await ensureDir(runDir); const runId = `run-${Date.now()}`; - await writeFile( + await atomicWrite( join(runDir, `${runId}.json`), - JSON.stringify({ id: runId, ...runData, completedAt: new Date().toISOString() }, null, 2) + { id: runId, ...runData, completedAt: new Date().toISOString() } ); cosEvents.emit(`${EVT}:run-complete`, { id, runId, name: agent.name, status: runData.status }); diff --git a/server/services/genome.js b/server/services/genome.js index 4915119b26..3fcd1d7225 100644 --- a/server/services/genome.js +++ b/server/services/genome.js @@ -1,7 +1,6 @@ -import { writeFile, unlink } from 'fs/promises'; import { join } from 'path'; import { randomUUID } from 'crypto'; -import { atomicWrite, PATHS, ensureDir, safeJSONParse, tryReadFile } from '../lib/fileUtils.js'; +import { atomicWrite, PATHS, ensureDir, safeJSONParse, tryReadFile, unlinkGuarded } from '../lib/fileUtils.js'; import { CURATED_MARKERS, MARKER_CATEGORIES, classifyGenotype, formatGenotype, resolveApoeHaplotype } from '../lib/curatedGenomeMarkers.js'; const GENOME_DIR = PATHS.meatspace; @@ -112,7 +111,7 @@ export async function uploadGenome(content, filename) { } // Save raw file - await writeFile(RAW_FILE, content); + await atomicWrite(RAW_FILE, content); // Cache the index snpIndex = index; @@ -369,8 +368,8 @@ export async function deleteMarker(id) { * Delete all genome data: raw file, metadata, and clear cache. */ export async function deleteGenome() { - await unlink(RAW_FILE).catch(() => {}); - await unlink(META_FILE).catch(() => {}); + await unlinkGuarded(RAW_FILE).catch(() => {}); + await unlinkGuarded(META_FILE).catch(() => {}); snpIndex = null; indexBuiltAt = 0; console.log('🧬 Genome data deleted'); diff --git a/server/services/googleAuth.js b/server/services/googleAuth.js index 3dcb875284..e4b0d1194a 100644 --- a/server/services/googleAuth.js +++ b/server/services/googleAuth.js @@ -1,5 +1,4 @@ import { OAuth2Client } from 'google-auth-library'; -import { writeFile } from 'fs/promises'; import { join } from 'path'; import { atomicWrite, ensureDir, PATHS, tryReadFile } from '../lib/fileUtils.js'; import { ServerError } from '../lib/errorHandler.js'; @@ -63,7 +62,7 @@ function attachTokenPersistence(client) { export async function clearAuth() { await ensureAuthDir(); - await writeFile(TOKENS_FILE, '{}').catch(() => {}); + await atomicWrite(TOKENS_FILE, {}).catch(() => {}); oAuth2Client = null; console.log('📅 Google OAuth tokens cleared'); } diff --git a/server/services/imageCleanTmpGc.js b/server/services/imageCleanTmpGc.js index 6f377d8b37..5bb2b8a4ea 100644 --- a/server/services/imageCleanTmpGc.js +++ b/server/services/imageCleanTmpGc.js @@ -14,9 +14,9 @@ * because a GPU clean completes and is fetched in seconds-to-minutes. */ -import { readdir, stat, unlink } from 'node:fs/promises'; +import { readdir, stat } from 'node:fs/promises'; import { join } from 'node:path'; -import { PATHS } from '../lib/fileUtils.js'; +import { PATHS, unlinkGuarded } from '../lib/fileUtils.js'; import { listJobs } from './mediaJobQueue/index.js'; import { createSweepScheduler } from './sweepScheduler.js'; @@ -92,7 +92,7 @@ export async function sweepImageCleanTmp({ keptYoung += 1; continue; } - const removed = await unlink(join(tmpDir, name)).then(() => true).catch(() => false); + const removed = await unlinkGuarded(join(tmpDir, name)).then(() => true).catch(() => false); if (removed) deleted += 1; } return { deleted, keptYoung, keptActive }; diff --git a/server/services/imageGen/agy.js b/server/services/imageGen/agy.js index 43189a0840..8d0b1a4deb 100644 --- a/server/services/imageGen/agy.js +++ b/server/services/imageGen/agy.js @@ -12,7 +12,7 @@ */ import { spawn } from '../../lib/childProcess.js'; -import { copyFile, mkdir, open, rename, rm, stat, unlink } from 'fs/promises'; +import { mkdir, open, stat } from 'fs/promises'; import { randomUUID } from 'crypto'; import { isAbsolute, join, resolve as pathResolve, sep } from 'path'; import { tmpdir } from 'os'; @@ -24,7 +24,7 @@ import { prepareAntigravityPrompt, } from '../../lib/antigravity.js'; import { bufferedSpawn, killProcessTree, prepareCliSpawn } from '../../lib/bufferedSpawn.js'; -import { atomicWrite, detectImageFormat, ensureDir, PATHS } from '../../lib/fileUtils.js'; +import { atomicWrite, copyFileGuarded, detectImageFormat, ensureDir, PATHS, rmGuarded, unlinkGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { autoCleanGeneratedImage } from '../../lib/imageClean.js'; import { killWithEscalation } from '../../lib/killWithEscalation.js'; @@ -320,7 +320,7 @@ async function runAgy(job, jobId, bin, args, { // scratch-dir spawn telling the child one consistent story about where it is. const proc = spawn(command, spawnArgs, { cwd: scratchDir, env: withSpawnCwdEnv(process.env, scratchDir), shell: false, stdio: ['ignore', 'pipe', 'pipe'] }); activeProcs.set(jobId, proc); - const removeScratch = () => rm(scratchDir, { recursive: true, force: true }).catch(() => {}); + const removeScratch = () => rmGuarded(scratchDir, { recursive: true, force: true }).catch(() => {}); let stdoutTail = ''; let stderrTail = ''; const timeoutTimer = setTimeout(() => { @@ -367,12 +367,11 @@ async function runAgy(job, jobId, bin, args, { return finalizeError(job, jobId, proc, `${fabricated} ${noImageReason(stdoutTail)}`); } if (harvested.format === 'png') { - await rename(stagingPath, outputPath).catch(async () => { - await copyFile(stagingPath, outputPath); - await unlink(stagingPath).catch(() => {}); - }); + await copyFileGuarded(stagingPath, outputPath); + await unlinkGuarded(stagingPath).catch(() => {}); } else { - await sharp(stagingPath).png().toFile(outputPath); + const pngBytes = await sharp(stagingPath).png().toBuffer(); + await atomicWrite(outputPath, pngBytes); } removeScratch(); // Degenerate-frame gate (#4173) — before the sidecar, so a decodable but diff --git a/server/services/imageGen/cleanGpu.js b/server/services/imageGen/cleanGpu.js index 11e51fdd52..499892e032 100644 --- a/server/services/imageGen/cleanGpu.js +++ b/server/services/imageGen/cleanGpu.js @@ -78,10 +78,9 @@ export async function enqueueGpuClean({ const initName = `init-${randomUUID()}.png`; const initAbsPath = join(PATHS.imageCleanTmp, initName); // Normalize to PNG so the runner's init-image decode is stable regardless of - // the source format, then write the raw bytes (atomicWrite is JSON-only). + // the source format, then write the raw bytes. const initPng = await sharp(initBuffer).png().toBuffer(); - const { writeFile } = await import('node:fs/promises'); - await writeFile(initAbsPath, initPng); + await atomicWrite(initAbsPath, initPng); // Conservative default for an arbitrary upload (no SynthID-bearing lineage // to key on) — mirrors the lightbox's resolveRegenStrengthDefault({}). @@ -116,8 +115,8 @@ export async function enqueueGpuClean({ // means the composite is skipped, never a failed enqueue. let hasMask = false; if (Buffer.isBuffer(originalBuffer) && Buffer.isBuffer(maskBuffer)) { - await writeFile(join(PATHS.imageCleanTmp, `${jobId}-original.png`), originalBuffer).catch(() => {}); - await writeFile(join(PATHS.imageCleanTmp, `${jobId}-mask.png`), maskBuffer).catch(() => {}); + await atomicWrite(join(PATHS.imageCleanTmp, `${jobId}-original.png`), originalBuffer).catch(() => {}); + await atomicWrite(join(PATHS.imageCleanTmp, `${jobId}-mask.png`), maskBuffer).catch(() => {}); hasMask = true; } await atomicWrite(cleanMetaPath(jobId), { hasMask, feather, initName }).catch(() => {}); diff --git a/server/services/imageGen/codex.js b/server/services/imageGen/codex.js index cf4e1b8519..229a898f22 100644 --- a/server/services/imageGen/codex.js +++ b/server/services/imageGen/codex.js @@ -30,12 +30,12 @@ */ import { spawn } from '../../lib/childProcess.js'; -import { copyFile, readFile, readdir, stat, writeFile } from 'fs/promises'; +import { readFile, readdir, stat } from 'fs/promises'; import { existsSync } from 'fs'; import { join } from 'path'; import { homedir } from 'os'; import { randomUUID } from 'crypto'; -import { atomicWrite, ensureDir, PATHS } from '../../lib/fileUtils.js'; +import { atomicWrite, copyFileGuarded, ensureDir, PATHS } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { autoCleanGeneratedImage } from '../../lib/imageClean.js'; import { imageGenEvents } from '../imageGenEvents.js'; @@ -413,9 +413,9 @@ async function runCodex(job, jobId, bin, args, outputPath, filename, meta, { cle return finalizeError(job, jobId, proc, noImageReason(stdoutTail)); } if (harvested.path) { - await copyFile(harvested.path, outputPath); + await copyFileGuarded(harvested.path, outputPath); } else { - await writeFile(outputPath, harvested.buffer); + await atomicWrite(outputPath, harvested.buffer); } // Degenerate-frame gate (#4173) — before the sidecar, so a decodable but // contentless canvas never becomes a gallery record. diff --git a/server/services/imageGen/external.js b/server/services/imageGen/external.js index cf0bc0b046..be667143d9 100644 --- a/server/services/imageGen/external.js +++ b/server/services/imageGen/external.js @@ -6,10 +6,9 @@ * Streams diffusion progress to imageGenEvents (bridged to Socket.IO). */ -import { writeFile } from 'fs/promises'; import { join } from 'path'; import { randomUUID } from 'crypto'; -import { ensureDir, PATHS } from '../../lib/fileUtils.js'; +import { atomicWrite, ensureDir, PATHS } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { fetchWithTimeout } from '../../lib/fetchWithTimeout.js'; import { autoCleanGeneratedImage } from '../../lib/imageClean.js'; @@ -157,7 +156,7 @@ export async function generateImage({ sdapiUrl, prompt, negativePrompt, width, h await ensureDir(PATHS.images); const filename = `${randomUUID()}.png`; const pngPath = join(PATHS.images, filename); - await writeFile(pngPath, Buffer.from(data.images[0], 'base64')); + await atomicWrite(pngPath, Buffer.from(data.images[0], 'base64')); // Auto-clean BEFORE the SSE complete fires so the URL the client opens // serves the cleaned bytes. External mode has no sidecar — pass null so // the helper just patches the PNG in place. diff --git a/server/services/imageGen/external.test.js b/server/services/imageGen/external.test.js index 8290988b2a..fcc79116d6 100644 --- a/server/services/imageGen/external.test.js +++ b/server/services/imageGen/external.test.js @@ -6,9 +6,9 @@ const { fetchWithTimeout, imageGenEvents } = vi.hoisted(() => ({ })); vi.mock('../../lib/fetchWithTimeout.js', () => ({ fetchWithTimeout })); -vi.mock('fs/promises', () => ({ writeFile: vi.fn() })); vi.mock('../../lib/fileUtils.js', () => ({ ensureDir: vi.fn(), + atomicWrite: vi.fn(), PATHS: { images: '/tmp/portos-external-image-test' }, })); vi.mock('../../lib/imageClean.js', () => ({ autoCleanGeneratedImage: vi.fn() })); diff --git a/server/services/imageGen/frameGuard.js b/server/services/imageGen/frameGuard.js index e186d6a746..a783afe6f8 100644 --- a/server/services/imageGen/frameGuard.js +++ b/server/services/imageGen/frameGuard.js @@ -21,9 +21,8 @@ * PNG lands, so each of them calls the gate from its own completion handler. */ -import { unlink } from 'fs/promises'; import { basename } from 'path'; -import { tryReadFile } from '../../lib/fileUtils.js'; +import { tryReadFile, unlinkGuarded } from '../../lib/fileUtils.js'; import { describeFrameStats, isDegenerateFrame } from '../../lib/imageFrameStats.js'; import { describeDegenerateFrame } from './noImageReason.js'; @@ -49,6 +48,6 @@ export async function degenerateFrameReason(pngPath) { */ export async function rejectDegenerateFrame(pngPath) { const reason = await degenerateFrameReason(pngPath); - if (reason) await unlink(pngPath).catch(() => {}); + if (reason) await unlinkGuarded(pngPath).catch(() => {}); return reason; } diff --git a/server/services/imageGen/grok.js b/server/services/imageGen/grok.js index 794bd359b4..828d6d4fe4 100644 --- a/server/services/imageGen/grok.js +++ b/server/services/imageGen/grok.js @@ -33,11 +33,11 @@ */ import { spawn } from '../../lib/childProcess.js'; -import { copyFile, mkdir, open, rename, rm, stat, unlink } from 'fs/promises'; +import { mkdir, open, stat } from 'fs/promises'; import { isAbsolute, join, resolve as pathResolve, sep } from 'path'; import { tmpdir } from 'os'; import { randomUUID } from 'crypto'; -import { atomicWrite, detectImageFormat, ensureDir, PATHS } from '../../lib/fileUtils.js'; +import { atomicWrite, copyFileGuarded, detectImageFormat, ensureDir, PATHS, rmGuarded, unlinkGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { autoCleanGeneratedImage } from '../../lib/imageClean.js'; import { imageGenEvents } from '../imageGenEvents.js'; @@ -321,7 +321,7 @@ async function runGrok(job, jobId, bin, args, { // scratch-dir spawn telling the child one consistent story about where it is. const proc = spawn(spawnBin, spawnArgs, { cwd: scratchDir, env: withSpawnCwdEnv(process.env, scratchDir), shell: false, stdio: [useStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'] }); activeProcs.set(jobId, proc); - const removeScratch = () => rm(scratchDir, { recursive: true, force: true }).catch(() => {}); + const removeScratch = () => rmGuarded(scratchDir, { recursive: true, force: true }).catch(() => {}); if (useStdin) { // POSIX: grok reads the prompt via --prompt-file /dev/stdin. EPIPE fires @@ -394,18 +394,14 @@ async function runGrok(job, jobId, bin, args, { return finalizeError(job, jobId, proc, `${fabricated} ${noImageReason(stdoutTail)}`); } if (harvested.format === 'png') { - // Move, not copy — the staging file is PortOS-owned and disposable, - // so rename is a metadata-only op when tmpdir and the gallery share - // a filesystem. copyFile+unlink is the cross-device (EXDEV) fallback. - await rename(stagingPath, outputPath).catch(async () => { - await copyFile(stagingPath, outputPath); - await unlink(stagingPath).catch(() => {}); - }); + await copyFileGuarded(stagingPath, outputPath); + await unlinkGuarded(stagingPath).catch(() => {}); } else { // Grok wrote a real image but not a PNG (jpeg/webp/gif) despite the // prompt. The gallery serves by extension and sidecars assume PNG, // so transcode rather than shipping mislabeled bytes. - await sharp(stagingPath).png().toFile(outputPath); + const pngBytes = await sharp(stagingPath).png().toBuffer(); + await atomicWrite(outputPath, pngBytes); } removeScratch(); // Degenerate-frame gate (#4173) — a decline that still emitted a flat diff --git a/server/services/imageGen/local.js b/server/services/imageGen/local.js index f3e23f4cca..24951cad07 100644 --- a/server/services/imageGen/local.js +++ b/server/services/imageGen/local.js @@ -14,12 +14,12 @@ import { spawn } from '../../lib/childProcess.js'; import sharp from 'sharp'; -import { writeFile, readFile, readdir, stat, unlink, rm, mkdtemp } from 'fs/promises'; +import { readFile, readdir, stat, mkdtemp } from 'fs/promises'; import { existsSync, watch as fsWatch } from 'fs'; import { join, dirname, resolve as resolvePath, sep as PATH_SEP, basename } from 'path'; import { tmpdir } from 'os'; import { randomUUID } from 'crypto'; -import { atomicWrite, assertSafeFilename, detectImageFormat, ensureDir, listDirectoryByExtension, PATHS, safeJSONParse, resolveImageInputPath, tryReadFile } from '../../lib/fileUtils.js'; +import { atomicWrite, assertSafeFilename, detectImageFormat, ensureDir, listDirectoryByExtension, PATHS, safeJSONParse, resolveImageInputPath, tryReadFile, rmGuarded, unlinkGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { autoCleanGeneratedImage } from '../../lib/imageClean.js'; import { rejectDegenerateFrame } from './frameGuard.js'; @@ -668,7 +668,7 @@ export async function generateImage({ pythonPath, prompt = '', negativePrompt = const heavyClaim = await claimHeavyLocalJob({ kind: 'local image generation', id: jobId }); if (!heavyClaim.ok) { jobs.delete(jobId); - await rm(stepwiseDir, { recursive: true, force: true }); + await rmGuarded(stepwiseDir, { recursive: true, force: true }); throw new ServerError(heavyClaim.message, { status: 409, code: 'HEAVY_LOCAL_JOB_BUSY', context: { holder: heavyClaim.holder } }); } const releaseHeavyClaim = () => heavyClaim.release() @@ -700,7 +700,7 @@ export async function generateImage({ pythonPath, prompt = '', negativePrompt = if (!claimHandedOff) { await releaseHeavyClaim(); jobs.delete(jobId); - await rm(stepwiseDir, { recursive: true, force: true }).catch(() => {}); + await rmGuarded(stepwiseDir, { recursive: true, force: true }).catch(() => {}); } throw err; } @@ -723,7 +723,7 @@ export async function generateImage({ pythonPath, prompt = '', negativePrompt = activeProcess = null; activeJob = null; void releaseHeavyClaim(); - rm(stepwiseDir, { recursive: true, force: true }).catch(() => {}); + rmGuarded(stepwiseDir, { recursive: true, force: true }).catch(() => {}); closeJobAfterDelay(jobs, jobId); }); @@ -908,7 +908,7 @@ export async function generateImage({ pythonPath, prompt = '', negativePrompt = activeJob = null; void releaseHeavyClaim(); if (watcher) { try { watcher.close(); } catch { /* ignore */ } } - rm(stepwiseDir, { recursive: true, force: true }).catch(() => {}); + rmGuarded(stepwiseDir, { recursive: true, force: true }).catch(() => {}); // Degenerate-frame gate (#4173): a runner can exit 0 having written a PNG // that decodes fine and holds no content (a solid-black frame from a run // that produced nothing). Fail the job instead of saving a black tile — @@ -1008,7 +1008,7 @@ export async function generateImage({ pythonPath, prompt = '', negativePrompt = .toBuffer() .catch((err) => { console.warn(`⚠️ Regen upscale failed for ${filename}: ${err?.message || err}`); return null; }); if (resized) { - await writeFile(outputPath, resized).catch(() => {}); + await atomicWrite(outputPath, resized).catch(() => {}); meta.renderWidth = meta.width; meta.renderHeight = meta.height; meta.width = targetW; @@ -1143,7 +1143,7 @@ export async function saveUploadedGalleryImage(base64Data) { const png = await sharp(buffer, { limitInputPixels: MAX_GALLERY_UPLOAD_PIXELS }).rotate().png().toBuffer(); const filename = `upload-${randomUUID().slice(0, 8)}.png`; await ensureDir(PATHS.images); - await writeFile(join(PATHS.images, filename), png); + await atomicWrite(join(PATHS.images, filename), png); console.log(`📥 Saved uploaded gallery image: ${filename} (${(png.length / 1024).toFixed(0)}KB PNG, from ${detected.mime})`); return { filename, path: `/data/images/${filename}` }; } @@ -1172,9 +1172,9 @@ export async function listGallery() { export async function deleteImage(filename) { assertGalleryFilename(filename); - await unlink(join(PATHS.images, filename)).catch(() => {}); - await unlink(join(PATHS.images, filename.replace('.png', '.metadata.json'))).catch(() => {}); - await unlink(join(PATHS.images, `${filename}.metadata.json`)).catch(() => {}); + await unlinkGuarded(join(PATHS.images, filename)).catch(() => {}); + await unlinkGuarded(join(PATHS.images, filename.replace('.png', '.metadata.json'))).catch(() => {}); + await unlinkGuarded(join(PATHS.images, `${filename}.metadata.json`)).catch(() => {}); // Drop the derived index row with the file (#2738). Without this the row // survives until the next boot reconcile, so anything counting the index // (the Character sheet's Auteur skill / Media Assets tile) reads high in diff --git a/server/services/imageGen/prepareParams.js b/server/services/imageGen/prepareParams.js index 2ed6cf307b..65efee659a 100644 --- a/server/services/imageGen/prepareParams.js +++ b/server/services/imageGen/prepareParams.js @@ -22,11 +22,10 @@ * middleware translates it to a 4xx response. */ -import { copyFile, unlink } from 'fs/promises'; import { randomUUID } from 'crypto'; import { join } from 'node:path'; import { ServerError } from '../../lib/errorHandler.js'; -import { PATHS, ensureDir, resolveGalleryImage } from '../../lib/fileUtils.js'; +import { PATHS, ensureDir, resolveGalleryImage, copyFileGuarded, unlinkGuarded } from '../../lib/fileUtils.js'; import { getSettings } from '../settings.js'; import { IMAGE_GEN_MODE, resolveImageCleaners } from './index.js'; import { editIncapableModeError, isEditCapableMode, modeLabel } from './modes.js'; @@ -123,7 +122,7 @@ export async function prepareGenerateParams({ data, files, referenceImageFields const cleanupReqFilesTemp = () => { if (!files) return; for (const f of Object.values(files)) { - if (f?.path) unlink(f.path).catch(() => {}); + if (f?.path) unlinkGuarded(f.path).catch(() => {}); } }; @@ -137,7 +136,7 @@ export async function prepareGenerateParams({ data, files, referenceImageFields const stagedRefPaths = []; const cleanupStagedAndTemp = () => { cleanupReqFilesTemp(); - for (const p of stagedRefPaths) unlink(p).catch(() => {}); + for (const p of stagedRefPaths) unlinkGuarded(p).catch(() => {}); }; // Resolve the effective backend BEFORE staging reference uploads — an @@ -263,7 +262,7 @@ export async function prepareGenerateParams({ data, files, referenceImageFields // on every i2i/edit render. The runner re-anchors init paths through // resolveImageInputPath, which accepts the refs dir. initImagePath = join(PATHS.imageRefs, initFilename); - await copyFile(initUpload.path, initImagePath); + await copyFileGuarded(initUpload.path, initImagePath); stagedRefPaths.push(initImagePath); uploadedTempPaths.push(initUpload.path); } else if (data.initImageFile) { @@ -300,7 +299,7 @@ export async function prepareGenerateParams({ data, files, referenceImageFields const ext = MIME_TO_EXT[(upload.mimetype || '').toLowerCase()] || '.png'; const refFilename = `ref-${randomUUID()}${ext}`; const refPath = join(PATHS.imageRefs, refFilename); - await copyFile(upload.path, refPath); + await copyFileGuarded(upload.path, refPath); stagedRefPaths.push(refPath); uploadedTempPaths.push(upload.path); referenceImagePaths.push(refPath); diff --git a/server/services/imageGen/variants.js b/server/services/imageGen/variants.js index 9d069a90f0..bc7c46c490 100644 --- a/server/services/imageGen/variants.js +++ b/server/services/imageGen/variants.js @@ -18,7 +18,7 @@ * The route handlers become: validate → read source → call helper → res.json. */ -import { readFile, writeFile } from 'fs/promises'; +import { readFile } from 'fs/promises'; import { join } from 'node:path'; import { ServerError } from '../../lib/errorHandler.js'; import { atomicWrite, PATHS } from '../../lib/fileUtils.js'; @@ -99,7 +99,7 @@ export async function persistVariant({ const untimedVariantMeta = omitRenderTiming(variantMeta); await Promise.all([ - writeFile(outPath, data), + atomicWrite(outPath, data), atomicWrite(sidecarPath, untimedVariantMeta), ]); diff --git a/server/services/imageRefsGc.js b/server/services/imageRefsGc.js index 7d63415508..b6d06311a7 100644 --- a/server/services/imageRefsGc.js +++ b/server/services/imageRefsGc.js @@ -36,9 +36,9 @@ * catches handler failures and records them in scheduler history. */ -import { readdir, stat, unlink } from 'fs/promises'; +import { readdir, stat } from 'fs/promises'; import { join, basename } from 'path'; -import { PATHS, tryReadFile, safeJSONParse } from '../lib/fileUtils.js'; +import { PATHS, tryReadFile, safeJSONParse, unlinkGuarded } from '../lib/fileUtils.js'; import { createSweepScheduler } from './sweepScheduler.js'; // `init-` / `ref-` with a decodable image extension — the exact @@ -129,7 +129,7 @@ export async function sweepOrphanRefImages({ } // Count only a genuine removal so the summary log can't over-report. A // concurrent vanish (ENOENT) or any unlink failure leaves `deleted` untouched. - const removed = await unlink(join(refsDir, name)).then(() => true).catch(() => false); + const removed = await unlinkGuarded(join(refsDir, name)).then(() => true).catch(() => false); if (removed) deleted += 1; } diff --git a/server/services/imageTo3d/models.genericDispatch.test.js b/server/services/imageTo3d/models.genericDispatch.test.js index d9443fdcca..95a6743f52 100644 --- a/server/services/imageTo3d/models.genericDispatch.test.js +++ b/server/services/imageTo3d/models.genericDispatch.test.js @@ -16,6 +16,8 @@ vi.mock('../../lib/fileUtils.js', () => ({ PATHS: { imageTo3d: '/mock/data/image-to-3d' }, resolveGalleryImage: vi.fn((filename) => `/mock/data/images/${filename}`), ensureDir: vi.fn(() => Promise.resolve()), + rmGuarded: vi.fn(() => Promise.resolve()), + writeFileGuarded: vi.fn(() => Promise.resolve()), })); vi.mock('./targets.js', () => ({ diff --git a/server/services/imageTo3d/models.js b/server/services/imageTo3d/models.js index 40c01eb3df..81331f28a7 100644 --- a/server/services/imageTo3d/models.js +++ b/server/services/imageTo3d/models.js @@ -17,9 +17,9 @@ import { randomUUID } from 'crypto'; import { join } from 'node:path'; -import { rm, access, writeFile } from 'node:fs/promises'; +import { access } from 'node:fs/promises'; import { ServerError } from '../../lib/errorHandler.js'; -import { PATHS, resolveGalleryImage, ensureDir } from '../../lib/fileUtils.js'; +import { PATHS, resolveGalleryImage, ensureDir, rmGuarded, writeFileGuarded } from '../../lib/fileUtils.js'; import { claimHeavyLocalJob } from '../../lib/heavyJobClaim.js'; import { prepareLocalMemory, gpuBlockersMessage } from '../localMemory.js'; import { slugifyForFilename } from '../../lib/civitai.js'; @@ -98,7 +98,7 @@ const usdzDiskPath = (id) => join(recordDir(id), 'model.usdz'); * not the render got far enough to emit a file. */ async function cleanupRenderDir(id) { - await rm(recordDir(id), { recursive: true, force: true }) + await rmGuarded(recordDir(id), { recursive: true, force: true }) .catch((err) => console.error(`❌ Image-to-3D cleanup failed for ${id}: ${err.message}`)); } @@ -286,7 +286,7 @@ async function executeRender({ id, operationId, adapter, sourcePath, caps, optio }), }; }, { includeDeleted: true }); - await rm(usdzDiskPath(id), { force: true }) + await rmGuarded(usdzDiskPath(id), { force: true }) .catch((err) => console.error(`❌ Image-to-3D stale USDZ cleanup failed for ${id}: ${err.message}`)); console.log(`🧊 Image-to-3D mesh ready: ${id}`); } catch (error) { @@ -518,7 +518,7 @@ export async function saveModelUsdz(id, bytes) { throw new ServerError('Payload is not a USDZ archive', { status: 400, code: 'USDZ_INVALID' }); } await ensureDir(recordDir(id)); - await writeFile(usdzDiskPath(id), bytes); + await writeFileGuarded(usdzDiskPath(id), bytes); console.log(`🥽 Image-to-3D stored AR export for ${id} (${bytes.length} bytes)`); return store.mutateModel(id, (current) => ({ ...current, diff --git a/server/services/imageTo3d/models.test.js b/server/services/imageTo3d/models.test.js index f8b42d3f21..1a0a319d78 100644 --- a/server/services/imageTo3d/models.test.js +++ b/server/services/imageTo3d/models.test.js @@ -1,8 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { posixPath } from '../../lib/testHelper.js'; -vi.mock('node:fs/promises', async (importOriginal) => ({ - ...(await importOriginal()), +const { rm, writeFile } = vi.hoisted(() => ({ rm: vi.fn(() => Promise.resolve()), writeFile: vi.fn(() => Promise.resolve()), })); @@ -13,6 +12,8 @@ vi.mock('../../lib/fileUtils.js', () => ({ filename === 'missing.png' ? null : `/mock/data/images/${filename}` )), ensureDir: vi.fn(() => Promise.resolve()), + rmGuarded: rm, + writeFileGuarded: writeFile, })); vi.mock('./targets.js', () => ({ @@ -66,7 +67,6 @@ vi.mock('./db.js', () => ({ recoverInterruptedModels: vi.fn(), })); -import { rm, writeFile } from 'node:fs/promises'; import { ensureDir } from '../../lib/fileUtils.js'; import { resolveTarget, renderOptionSupportFor } from './targets.js'; import { isTrellis2Installed, runTrellis2Generate } from './trellis2.js'; diff --git a/server/services/jira.js b/server/services/jira.js index 87f5aa6d87..4a711b72ce 100644 --- a/server/services/jira.js +++ b/server/services/jira.js @@ -8,7 +8,7 @@ import { createBoundedStateMap } from '../lib/boundedStateMap.js'; import { createHttpClient } from '../lib/httpClient.js'; import { createSingleFlight } from '../lib/singleFlight.js'; import path from 'path'; -import { ensureDir, PATHS, readJSONFile } from '../lib/fileUtils.js'; +import { atomicWrite, ensureDir, PATHS, readJSONFile } from '../lib/fileUtils.js'; import { hostFromOriginUrl } from '../lib/workTracker.js'; import { countConfiguredInstances } from '../lib/instanceFeatureRegistry.js'; @@ -74,12 +74,7 @@ export async function getInstances() { * Save JIRA instances configuration */ export async function saveInstances(config) { - await ensureDir(path.dirname(JIRA_CONFIG_FILE)); - await fs.writeFile( - JIRA_CONFIG_FILE, - JSON.stringify(config, null, 2), - 'utf-8' - ); + await atomicWrite(JIRA_CONFIG_FILE, config); } /** diff --git a/server/services/jira.test.js b/server/services/jira.test.js index 7007ff354c..847748ac35 100644 --- a/server/services/jira.test.js +++ b/server/services/jira.test.js @@ -1,6 +1,14 @@ import fs from 'fs/promises'; import { describe, it, expect, vi, afterEach } from 'vitest'; -import { +import { createTempDataRoot, makePathsProxy } from '../lib/mockPathsDataRoot.js'; + +const tempRoot = createTempDataRoot('portos-jira-'); + +vi.mock('../lib/fileUtils.js', async () => { + const actual = await vi.importActual('../lib/fileUtils.js'); + return makePathsProxy(actual, { dataRoot: tempRoot }); +}); +const { addIssuesToSprint, buildColumnsFromBoardConfig, buildColumnsFromStatuses, @@ -17,7 +25,7 @@ import { resolveCustomFieldIds, updateTicket, upsertInstance -} from './jira.js'; +} = await import('./jira.js'); describe('isCloudInstance', () => { it('treats *.atlassian.net hosts as Cloud', () => { diff --git a/server/services/loraDatasetGenerate.js b/server/services/loraDatasetGenerate.js index 580a5451f3..7ab67a4171 100644 --- a/server/services/loraDatasetGenerate.js +++ b/server/services/loraDatasetGenerate.js @@ -15,10 +15,10 @@ * timeout. See that module for the rationale on each piece. */ -import { copyFile, readFile } from 'fs/promises'; +import { readFile } from 'fs/promises'; import { join, basename } from 'path'; import sharp from 'sharp'; -import { PATHS, ensureDir, shortId } from '../lib/fileUtils.js'; +import { PATHS, ensureDir, shortId, copyFileGuarded } from '../lib/fileUtils.js'; import { ServerError } from '../lib/errorHandler.js'; import { v4 as uuidv4 } from '../lib/uuid.js'; import { buildVariationMatrix } from '../lib/loraDataset.js'; @@ -244,7 +244,7 @@ async function onRenderComplete({ datasetId, imageId, file, sourceFilename }) { } await ensureDir(datasetImagesDir(datasetId)); const srcPath = join(PATHS.images, basename(sourceFilename)); - await copyFile(srcPath, datasetImagePath(datasetId, file)); + await copyFileGuarded(srcPath, datasetImagePath(datasetId, file)); await setImageStatus(datasetId, imageId, 'ready'); console.log(`📸 Dataset ${shortId(datasetId)} ← render ${file}`); } diff --git a/server/services/loraDatasets.js b/server/services/loraDatasets.js index c9a5448180..8412d21c3f 100644 --- a/server/services/loraDatasets.js +++ b/server/services/loraDatasets.js @@ -13,11 +13,10 @@ * pure helpers in `lib/loraDataset.js`. */ -import { copyFile, unlink } from 'fs/promises'; import { existsSync } from 'fs'; import { join, basename } from 'path'; import sharp from 'sharp'; -import { PATHS, ensureDir } from '../lib/fileUtils.js'; +import { PATHS, ensureDir, copyFileGuarded, unlinkGuarded } from '../lib/fileUtils.js'; import { assertGalleryFilename } from './imageGen/local.js'; import { createCollectionStore } from '../lib/collectionStore.js'; import { @@ -345,7 +344,7 @@ export async function addUploadedImage(id, { tmpPath, originalname = '' }) { const file = `${imageId}.png`; await ensureDir(datasetImagesDir(id)); const destPath = datasetImagePath(id, file); - const cleanup = () => unlink(tmpPath).catch(() => {}); + const cleanup = () => unlinkGuarded(tmpPath).catch(() => {}); const info = await sharp(tmpPath).rotate().png().toFile(destPath).catch(async (err) => { await cleanup(); throw new ServerError( @@ -406,7 +405,7 @@ export async function importGalleryImages(id, { filenames = [] } = {}) { if (failure) { // All transcodes have settled now, so `written` is complete — no late write // can re-orphan a file after this cleanup. - await Promise.all(written.map((p) => unlink(p).catch(() => {}))); + await Promise.all(written.map((p) => unlinkGuarded(p).catch(() => {}))); throw failure.reason; } const entries = results.map((r) => r.value); @@ -488,7 +487,7 @@ export async function deleteImage(id, imageId) { } return { ...current, images: current.images.filter((img) => img.id !== imageId) }; }); - await unlink(datasetImagePath(id, removed.file)).catch(() => {}); + await unlinkGuarded(datasetImagePath(id, removed.file)).catch(() => {}); return { ok: true, imageId }; } @@ -525,7 +524,7 @@ export async function reconcileRenderingImages(id, { jobLookup = getJob } = {}) await ensureDir(datasetImagesDir(id)); // basename() so a hand-edited media-jobs.json filename can't traverse // out of the gallery (mirrors onRenderComplete in loraDatasetGenerate). - await copyFile(join(PATHS.images, basename(res.sourceFilename)), datasetImagePath(id, img.file)) + await copyFileGuarded(join(PATHS.images, basename(res.sourceFilename)), datasetImagePath(id, img.file)) .catch((err) => { console.error(`❌ Dataset ${id} reconcile copy failed [${imageId}]: ${err?.message}`); resolutions.set(imageId, { status: 'failed' }); diff --git a/server/services/loraTraining/index.js b/server/services/loraTraining/index.js index 522d77a080..01dc05076a 100644 --- a/server/services/loraTraining/index.js +++ b/server/services/loraTraining/index.js @@ -15,10 +15,9 @@ import { spawn } from '../../lib/childProcess.js'; import { existsSync } from 'fs'; -import { copyFile, writeFile } from 'fs/promises'; import { join, basename, dirname } from 'path'; import { platform } from 'os'; -import { PATHS, ensureDir, atomicWrite, shortId } from '../../lib/fileUtils.js'; +import { PATHS, ensureDir, atomicWrite, shortId, copyFileGuarded, writeFileGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { v4 as uuidv4 } from '../../lib/uuid.js'; import { hfChildEnv } from '../hfToken.js'; @@ -591,12 +590,12 @@ export async function runTraining({ jobId, runId, pythonPath = null, resumeCheck await ensureDir(dataDir); for (let i = 0; i < manifest.images.length; i += 1) { const stem = String(i + 1).padStart(4, '0'); - await copyFile(manifest.images[i].path, join(dataDir, `${stem}.png`)); - await writeFile(join(dataDir, `${stem}.txt`), `${manifest.images[i].caption}\n`); + await copyFileGuarded(manifest.images[i].path, join(dataDir, `${stem}.png`)); + await writeFileGuarded(join(dataDir, `${stem}.txt`), `${manifest.images[i].caption}\n`); } if ((run.params?.sampleEvery ?? TRAINING_DEFAULTS.sampleEvery) > 0) { const samplePrompt = run.params?.samplePrompt || `${run.triggerWord} portrait, neutral background`; - await writeFile(join(dataDir, 'preview_1.txt'), `${samplePrompt}\n`); + await writeFileGuarded(join(dataDir, 'preview_1.txt'), `${samplePrompt}\n`); } // output_path must NOT pre-exist — mflux appends a timestamp suffix to // an existing dir (its new_folder behavior), which would break the @@ -1040,7 +1039,7 @@ async function registerTrainedLora({ }) { await ensureDir(PATHS.loras); const dest = join(PATHS.loras, filename); - await writeFile(dest, buffer); + await writeFileGuarded(dest, buffer); const sizeBytes = buffer.length; // bytes written === on-disk size; no stat round-trip const sidecar = buildTrainedSidecar({ run, result, filename, previewImageUrl, sizeBytes, selectedStep, autoSelected, @@ -1183,7 +1182,7 @@ async function ensureCheckpointPreview(run, step, loraFilename) { if (!existsSync(renderedPath)) { console.error(`⚠️ training [${shortId(runId)}] preview render produced no file`); return; } await ensureDir(samplesDir); - await copyFile(renderedPath, dest); + await copyFileGuarded(renderedPath, dest); // Join is by step, sourced from artifacts.samples — append so listRunCheckpoints // picks it up. updateRun's function form merges against the freshest record. await runsDb.updateRun(runId, (current) => { diff --git a/server/services/loras.js b/server/services/loras.js index 4b5298c7b3..1e72456f74 100644 --- a/server/services/loras.js +++ b/server/services/loras.js @@ -17,7 +17,7 @@ */ import { existsSync } from 'fs'; -import { link, readFile, rename, rm, stat, unlink } from 'fs/promises'; +import { link, readFile, rename, stat } from 'fs/promises'; import { basename, join } from 'path'; import { ServerError } from '../lib/errorHandler.js'; import { @@ -28,7 +28,7 @@ import { siblingDownloadMeta, streamResumableDownload, } from '../lib/downloadPreflight.js'; -import { atomicWrite, assertSafeFilename, ensureDir, listDirectoryByExtension, sha256File, PATHS } from '../lib/fileUtils.js'; +import { atomicWrite, assertSafeFilename, ensureDir, listDirectoryByExtension, sha256File, PATHS, rmGuarded, unlinkGuarded } from '../lib/fileUtils.js'; import { verifySafetensorsStructure } from '../lib/hfCache.js'; import { isPlainObject } from '../lib/objects.js'; import { readCachedLoraEffectReport } from '../lib/loraEffect.js'; @@ -338,8 +338,8 @@ export const deleteLora = async (filename) => { (contents) => contents, (err) => (err.code === 'ENOENT' ? null : Promise.reject(err)), ); - if (metadata !== null) await rm(metadataPath); - const modelRemovalError = await rm(filePath).then(() => null, (err) => err); + if (metadata !== null) await rmGuarded(metadataPath); + const modelRemovalError = await rmGuarded(filePath).then(() => null, (err) => err); if (modelRemovalError) { if (metadata !== null) await atomicWrite(metadataPath, metadata); throw modelRemovalError; @@ -491,7 +491,7 @@ const downloadToFile = async (url, destPath, { fetchImpl = fetch, headers = {} , // Reaching here means the stream completed successfully — every branch // below either moves or deletes tmpPath outright, never leaves it for a // future resume, so the sidecar describing it is equally done. - await rm(etagPathFor(destPath), { force: true }).catch(() => {}); + await rmGuarded(etagPathFor(destPath), { force: true }).catch(() => {}); if (onProgress) onProgress(lastTick); // Atomic no-clobber finalize: `link` is POSIX-atomic and fails with EEXIST // when destPath already exists (concurrent install that snuck past our @@ -501,11 +501,11 @@ const downloadToFile = async (url, destPath, { fetchImpl = fetch, headers = {} , // portable option on those platforms. const linkErr = await link(tmpPath, destPath).catch((e) => e); if (!linkErr) { - await unlink(tmpPath).catch(() => {}); + await unlinkGuarded(tmpPath).catch(() => {}); return; } if (linkErr.code === 'EEXIST') { - await rm(tmpPath, { force: true }).catch(() => {}); + await rmGuarded(tmpPath, { force: true }).catch(() => {}); const basename_ = basename(destPath); throw new ServerError( `Already installed: ${basename_}. Delete it first or pick a different version.`, @@ -518,7 +518,7 @@ const downloadToFile = async (url, destPath, { fetchImpl = fetch, headers = {} , // late-arriving dest as CIVITAI_ALREADY_INSTALLED, matching the EEXIST // path above. if (existsSync(destPath)) { - await rm(tmpPath, { force: true }).catch(() => {}); + await rmGuarded(tmpPath, { force: true }).catch(() => {}); const basename_ = basename(destPath); throw new ServerError( `Already installed: ${basename_}. Delete it first or pick a different version.`, @@ -526,7 +526,7 @@ const downloadToFile = async (url, destPath, { fetchImpl = fetch, headers = {} , ); } await rename(tmpPath, destPath).catch(async (err) => { - await rm(tmpPath, { force: true }).catch(() => {}); + await rmGuarded(tmpPath, { force: true }).catch(() => {}); throw err; }); }; @@ -547,7 +547,7 @@ const verifyDownloadedLora = async (destPath, { expectedSha256 = null, source = const st = await stat(destPath).catch(() => null); const structural = await verifySafetensorsStructure(destPath, st?.size ?? 0); if (!structural.ok) { - await rm(destPath, { force: true }).catch(() => {}); + await rmGuarded(destPath, { force: true }).catch(() => {}); throw new ServerError( `${label} LoRA download is corrupt (${structural.reason}) — the partial file was deleted. Retry the install.`, { status: 502, code }, @@ -559,7 +559,7 @@ const verifyDownloadedLora = async (destPath, { expectedSha256 = null, source = if (/^[0-9a-f]{64}$/.test(want)) { const actual = await sha256File(destPath).catch(() => null); if (actual && actual.toLowerCase() !== want) { - await rm(destPath, { force: true }).catch(() => {}); + await rmGuarded(destPath, { force: true }).catch(() => {}); throw new ServerError( `${label} LoRA failed SHA-256 verification (expected ${want.slice(0, 12)}…, got ${actual.slice(0, 12)}…) — the file was deleted. Retry the install.`, { status: 502, code }, diff --git a/server/services/mediaSketches.js b/server/services/mediaSketches.js index ec4b0ac60b..d1de27b980 100644 --- a/server/services/mediaSketches.js +++ b/server/services/mediaSketches.js @@ -27,8 +27,8 @@ import { join } from 'path'; import { randomUUID } from 'crypto'; -import { unlink, access } from 'fs/promises'; -import { PATHS, atomicWrite, readJSONFile, ensureDir, tryReadFile } from '../lib/fileUtils.js'; +import { access } from 'fs/promises'; +import { PATHS, atomicWrite, readJSONFile, ensureDir, tryReadFile, unlinkGuarded } from '../lib/fileUtils.js'; import { isValidKey as isValidMediaKey, parseKey } from '../lib/mediaItemKey.js'; const SKETCH_DIR = join(PATHS.data, 'media-sketches'); @@ -171,7 +171,7 @@ export async function saveSketch(key, input) { // A re-save that carries only vectors (no PNG) must drop any prior flattened // export, otherwise `hasPng:false` in the JSON disagrees with a stale // `.png` that getSketchPng() would keep streaming. - else await unlink(pngPathFor(key)).catch(() => {}); + else await unlinkGuarded(pngPathFor(key)).catch(() => {}); const record = { key, width: clean.width, @@ -187,6 +187,6 @@ export async function saveSketch(key, input) { /** Remove a key's sidecar (json + png). Idempotent. */ export async function removeSketch(key) { if (!isValidKey(key)) throw makeErr(`Invalid key: ${key}`, ERR_VALIDATION); - await unlink(jsonPathFor(key)).catch(() => {}); - await unlink(pngPathFor(key)).catch(() => {}); + await unlinkGuarded(jsonPathFor(key)).catch(() => {}); + await unlinkGuarded(pngPathFor(key)).catch(() => {}); } diff --git a/server/services/mediaSketches.test.js b/server/services/mediaSketches.test.js index fcdcfb5daa..bbc6f761f1 100644 --- a/server/services/mediaSketches.test.js +++ b/server/services/mediaSketches.test.js @@ -9,6 +9,7 @@ vi.mock('../lib/fileUtils.js', () => ({ atomicWrite: vi.fn(async (path, data) => { fileStore.set(path, data); }), readJSONFile: vi.fn(async (path, fallback) => (fileStore.has(path) ? fileStore.get(path) : fallback)), tryReadFile: vi.fn(async (path) => (fileStore.has(path) ? fileStore.get(path) : null)), + unlinkGuarded: vi.fn(async (path) => { fileStore.delete(path); }), })); vi.mock('fs/promises', () => ({ diff --git a/server/services/memory.test.js b/server/services/memory.test.js index 6cb70888ef..9993fe6801 100644 --- a/server/services/memory.test.js +++ b/server/services/memory.test.js @@ -41,6 +41,7 @@ vi.mock('../lib/fileUtils.js', () => ({ ensureDirs: vi.fn(), readJSONFile: vi.fn(), atomicWrite: vi.fn().mockResolvedValue(undefined), + rmGuarded: vi.fn().mockResolvedValue(undefined), PATHS: { memory: '/tmp/test/memory' } })); @@ -73,9 +74,8 @@ vi.mock('./memoryConfig.js', () => ({ decrementAgentPendingApproval: vi.fn().mockResolvedValue(undefined) })); -import { rm } from 'fs/promises'; import { existsSync } from 'fs'; -import { ensureDir, ensureDirs, readJSONFile, atomicWrite } from '../lib/fileUtils.js'; +import { ensureDir, ensureDirs, readJSONFile, atomicWrite, rmGuarded } from '../lib/fileUtils.js'; import * as memoryBM25 from './memoryBM25.js'; import * as notifications from './notifications.js'; import { findTopK, findAboveThreshold, clusterBySimilarity } from '../lib/vectorMath.js'; @@ -665,7 +665,7 @@ describe('memory service', () => { const result = await deleteMemory('mem-1'); expect(result).toEqual({ success: true, id: 'mem-1' }); - expect(rm).not.toHaveBeenCalled(); + expect(rmGuarded).not.toHaveBeenCalled(); // The memory should have been saved with archived status const memorySaveCall = atomicWrite.mock.calls.find(c => c[0].includes('memory.json')); expect(memorySaveCall).toBeDefined(); @@ -681,7 +681,7 @@ describe('memory service', () => { const result = await deleteMemory('mem-1', true); expect(result).toEqual({ success: true, id: 'mem-1' }); - expect(rm).toHaveBeenCalled(); + expect(rmGuarded).toHaveBeenCalled(); expect(memoryBM25.removeMemoryFromIndex).toHaveBeenCalledWith('mem-1'); }); @@ -785,7 +785,7 @@ describe('memory service', () => { const result = await rejectMemory('mem-1'); expect(result.success).toBe(true); - expect(rm).toHaveBeenCalled(); + expect(rmGuarded).toHaveBeenCalled(); expect(notifications.removeByMetadata).toHaveBeenCalledWith('memoryId', 'mem-1'); expect(decrementAgentPendingApproval).toHaveBeenCalledWith('agent-1'); }); diff --git a/server/services/memoryStore.js b/server/services/memoryStore.js index f7628c4cf9..0d0af84ecc 100644 --- a/server/services/memoryStore.js +++ b/server/services/memoryStore.js @@ -8,10 +8,9 @@ * CRUD/search logic and events on top of these primitives. */ -import { rm } from 'fs/promises'; import { existsSync } from 'fs'; import { join, resolve, relative, isAbsolute } from 'path'; -import { ensureDir, ensureDirs, readJSONFile, atomicWrite, PATHS } from '../lib/fileUtils.js'; +import { ensureDir, ensureDirs, readJSONFile, atomicWrite, rmGuarded, PATHS } from '../lib/fileUtils.js'; import { createMutex } from '../lib/asyncMutex.js'; const MEMORY_DIR = PATHS.memory; @@ -120,7 +119,7 @@ export async function deleteMemoryFiles(id) { } if (existsSync(memoryDir)) { - await rm(memoryDir, { recursive: true }); + await rmGuarded(memoryDir, { recursive: true }); } } diff --git a/server/services/missions.js b/server/services/missions.js index 0cf62df020..61a9a125d2 100644 --- a/server/services/missions.js +++ b/server/services/missions.js @@ -5,11 +5,11 @@ * Enables proactive task generation when user queue is empty. */ -import { promises as fs } from 'fs' +import { readdir } from 'fs/promises' import path from 'path' import { v4 as uuidv4 } from '../lib/uuid.js' import { cosEvents } from './cosEvents.js' -import { atomicWrite, safeJSONParse, ensureDir, PATHS, tryReadFile } from '../lib/fileUtils.js' +import { atomicWrite, safeJSONParse, ensureDir, PATHS, tryReadFile, unlinkGuarded } from '../lib/fileUtils.js' const DATA_DIR = PATHS.missions @@ -61,7 +61,7 @@ async function loadMissions() { await ensureDir(DATA_DIR) - const files = await fs.readdir(DATA_DIR).catch(() => []) + const files = await readdir(DATA_DIR).catch(() => []) // Read every mission file in parallel on cold load rather than serializing // one disk read at a time; the result is memoized in missionsCache below. @@ -432,7 +432,7 @@ async function getStats() { */ async function deleteMission(id) { const filePath = path.join(DATA_DIR, `${id}.json`) - await fs.unlink(filePath).catch(() => {}) + await unlinkGuarded(filePath).catch(() => {}) if (missionsCache) { missionsCache = missionsCache.filter(m => m.id !== id) diff --git a/server/services/moodBoard/pinterest.js b/server/services/moodBoard/pinterest.js index fabcd61f66..9a2ec4c856 100644 --- a/server/services/moodBoard/pinterest.js +++ b/server/services/moodBoard/pinterest.js @@ -17,10 +17,9 @@ */ import { createHash } from 'crypto'; -import { writeFile } from 'fs/promises'; import { join } from 'path'; import { ServerError } from '../../lib/errorHandler.js'; -import { PATHS, ensureDir, detectImageFormat } from '../../lib/fileUtils.js'; +import { PATHS, ensureDir, detectImageFormat, writeFileGuarded } from '../../lib/fileUtils.js'; import { normalizePinterestFeedUrl, parsePinterestRss } from '../../lib/pinterestFeed.js'; import { fetchPublicText, fetchPublicBinary } from '../../lib/safeUrlFetch.js'; import { emitRecordUpdated } from '../sharing/recordEvents.js'; @@ -57,7 +56,7 @@ async function downloadPinImage({ pinUrl, imageUrl, imageUrlOriginal }) { const fmt = detectImageFormat(res.buffer); if (!fmt) return null; const filename = `pinterest-${createHash('sha1').update(pinUrl).digest('hex').slice(0, 16)}${fmt.ext}`; - await writeFile(join(PATHS.images, filename), res.buffer); + await writeFileGuarded(join(PATHS.images, filename), res.buffer); return `/data/images/${filename}`; } diff --git a/server/services/obsidian.js b/server/services/obsidian.js index a462b9b75f..f417571403 100644 --- a/server/services/obsidian.js +++ b/server/services/obsidian.js @@ -6,11 +6,11 @@ * Notes stay in their original vault directories — PortOS indexes but doesn't copy. */ -import { writeFile, readdir, stat, unlink } from 'fs/promises'; +import { readdir, stat } from 'fs/promises'; import { existsSync, realpathSync } from 'fs'; import { join, relative, resolve, basename, dirname, extname, isAbsolute } from 'path'; import { v4 as uuidv4 } from '../lib/uuid.js'; -import { atomicWrite, ensureDir, readJSONFile, PATHS } from '../lib/fileUtils.js'; +import { atomicWrite, ensureDir, readJSONFile, PATHS, writeFileGuarded, unlinkGuarded } from '../lib/fileUtils.js'; import { ICLOUD_NOT_MATERIALIZED, isSuspectedDataless, materializeAndWait, readIfMaterialized } from '../lib/icloudFile.js'; import { escapeRegExp } from '../lib/textUtils.js'; @@ -364,7 +364,7 @@ export async function updateNote(vaultId, notePath, content, { force = false } = } } - await writeFile(fullPath, content, 'utf-8'); + await writeFileGuarded(fullPath, content, 'utf-8'); console.log(`📓 Updated note: ${notePath} in vault ${vault.name}`); return await getNote(vaultId, notePath, { includeBacklinks: false }); } @@ -398,7 +398,7 @@ export async function createNote(vaultId, notePath, content = '') { // iCloud containers holding 373 evicted files, macOS 26 / APFS), and the probe // `mortalLoomStore` used to carry for it was deleted rather than copied here. // See the "only ONE representation" section in server/lib/icloudFile.js. - await writeFile(fullPath, content, 'utf-8'); + await writeFileGuarded(fullPath, content, 'utf-8'); console.log(`📓 Created note: ${notePath} in vault ${vault.name}`); return await getNote(vaultId, notePath, { includeBacklinks: false }); } @@ -463,7 +463,7 @@ export async function deleteNote(vaultId, notePath) { // threadpool the way `updateNote`'s overwrite could, and guarding it would be // strictly worse than useless: the guard's own remedy is `materializeAndWait`, // i.e. downloading every byte of a file purely to throw it away. - await unlink(fullPath); + await unlinkGuarded(fullPath); console.log(`📓 Deleted note: ${notePath} from vault ${vault.name}`); return true; } diff --git a/server/services/persistentMindSupervisor.attachments.test.js b/server/services/persistentMindSupervisor.attachments.test.js index 02832a4cc5..ef3cccd760 100644 --- a/server/services/persistentMindSupervisor.attachments.test.js +++ b/server/services/persistentMindSupervisor.attachments.test.js @@ -39,6 +39,8 @@ vi.mock('../lib/fileUtils.js', () => ({ resolveScreenshot: mocks.resolveScreenshot, sanitizeFilename: mocks.sanitizeFilename, saveImageUpload: mocks.saveImageUpload, + unlinkGuarded: mocks.unlink, + writeFileGuarded: mocks.writeFile, })); vi.mock('fs/promises', async (importOriginal) => ({ diff --git a/server/services/persistentMindSupervisor.js b/server/services/persistentMindSupervisor.js index f48e3b3bc3..4216ea41be 100644 --- a/server/services/persistentMindSupervisor.js +++ b/server/services/persistentMindSupervisor.js @@ -10,7 +10,7 @@ */ import { randomUUID } from 'crypto'; -import { mkdir, readFile, readdir, stat, unlink, writeFile } from 'fs/promises'; +import { mkdir, readFile, readdir, stat } from 'fs/promises'; import { join } from 'path'; import { getDomainMode } from '../lib/domainAutonomy.js'; import { @@ -35,6 +35,8 @@ import { resolveScreenshot, sanitizeFilename, saveImageUpload, + unlinkGuarded, + writeFileGuarded, } from '../lib/fileUtils.js'; import { isDaemonRunning, loadState, saveState, withStateLock } from './cosState.js'; import { cosEvents, emitLog } from './cosEvents.js'; @@ -146,7 +148,7 @@ const pendingAttachmentMarkerPath = (attachmentId) => join( `${PENDING_ATTACHMENT_MARKER_PREFIX}${attachmentId}`, ); -const removePendingAttachmentMarker = async (attachmentId) => unlink(pendingAttachmentMarkerPath(attachmentId)).then( +const removePendingAttachmentMarker = async (attachmentId) => unlinkGuarded(pendingAttachmentMarkerPath(attachmentId)).then( () => true, (error) => { if (error?.code === 'ENOENT') return true; @@ -158,7 +160,7 @@ const removePendingAttachmentMarker = async (attachmentId) => unlink(pendingAtta const removeStoredFilename = async (filename) => { const filePath = resolveScreenshot(filename); if (!filePath) return true; - return unlink(filePath).then( + return unlinkGuarded(filePath).then( () => true, (error) => { if (error?.code === 'ENOENT') return true; @@ -238,7 +240,7 @@ const removeStoredAttachmentFile = async (attachment) => { return removeStoredFilename(attachment.filename); }; -const removeUploadAfterStateFailure = async (filePath, attachmentId) => unlink(filePath).then( +const removeUploadAfterStateFailure = async (filePath, attachmentId) => unlinkGuarded(filePath).then( async () => removePendingAttachmentMarker(attachmentId), (error) => { if (error?.code !== 'ENOENT') { @@ -340,7 +342,7 @@ export async function createPersistentMindAttachment({ filename, data } = {}) { // write but before the state record is saved, boot/activity cleanup can find // and reap the otherwise-unindexed file without ever scanning durable assets. await mkdir(PATHS.screenshots, { recursive: true }); - await writeFile(pendingAttachmentMarkerPath(attachmentId), '', { flag: 'wx' }); + await writeFileGuarded(pendingAttachmentMarkerPath(attachmentId), '', { flag: 'wx' }); const saved = await saveImageUpload(PATHS.screenshots, { filename: `mind-${attachmentId}-${originalName}`, data, diff --git a/server/services/pipeline/audio.js b/server/services/pipeline/audio.js index 9b5d967cd8..27cbd20293 100644 --- a/server/services/pipeline/audio.js +++ b/server/services/pipeline/audio.js @@ -14,10 +14,9 @@ * `engine:` prefix is interpreted as the active engine's voice. */ -import { writeFile } from 'fs/promises'; import { join } from 'path'; import { randomUUID } from 'crypto'; -import { PATHS, ensureDir } from '../../lib/fileUtils.js'; +import { PATHS, ensureDir, atomicWrite } from '../../lib/fileUtils.js'; import { synthesize, listVoices, VALID_ENGINES } from '../voice/tts.js'; import { ServerError } from '../../lib/errorHandler.js'; @@ -249,7 +248,7 @@ export async function synthesizeToFile({ text, voiceId, profileId, route = 'stud // UUID filename keeps two simultaneous renders from colliding; the line's // audioJobId-or-audioFilename binding lives in stages.audio.lines[]. const filename = `vo-${randomUUID()}.wav`; - await writeFile(join(PATHS.audio, filename), wav); + await atomicWrite(join(PATHS.audio, filename), wav); return { filename, latencyMs, diff --git a/server/services/pipeline/musicLibrary.js b/server/services/pipeline/musicLibrary.js index 787f3f3dc6..3e1e4dc159 100644 --- a/server/services/pipeline/musicLibrary.js +++ b/server/services/pipeline/musicLibrary.js @@ -14,9 +14,9 @@ * care which one wrote the bytes. */ -import { stat, copyFile, unlink } from 'fs/promises'; +import { stat } from 'fs/promises'; import { existsSync } from 'fs'; -import { join, extname, basename } from 'path'; +import { join, basename, extname } from 'path'; import { randomUUID } from 'crypto'; import { PATHS, @@ -24,6 +24,8 @@ import { assertSafeFilename, isSafeFilename, listDirectoryByExtension, + copyFileGuarded, + unlinkGuarded, } from '../../lib/fileUtils.js'; // Mirror of the sanitizer's MUSIC_SOURCES set in `services/pipeline/issues.js`. @@ -142,8 +144,8 @@ export async function importUploadedTrack(tempPath, originalName) { // os.tmpdir(), which may sit on a different filesystem (rename across // devices throws EXDEV on Linux). Copy works regardless; the temp file // unlink is best-effort cleanup. - await copyFile(tempPath, dest); - await unlink(tempPath).catch(() => {}); + await copyFileGuarded(tempPath, dest); + await unlinkGuarded(tempPath).catch(() => {}); const s = await stat(dest).catch(() => null); return { filename, sizeBytes: s?.size ?? 0 }; } @@ -175,7 +177,7 @@ export async function deleteMusicTrack(filename) { assertSafeMusicFilename(filename); const full = join(PATHS.music, filename); try { - await unlink(full); + await unlinkGuarded(full); return true; } catch (err) { if (err.code === 'ENOENT') return false; diff --git a/server/services/sharing/exporter.js b/server/services/sharing/exporter.js index 36b02869c4..c1f7ec4129 100644 --- a/server/services/sharing/exporter.js +++ b/server/services/sharing/exporter.js @@ -16,9 +16,9 @@ */ import { join, basename } from 'path'; -import { copyFile, readFile, writeFile, stat } from 'fs/promises'; +import { readFile, stat } from 'fs/promises'; import { existsSync } from 'fs'; -import { PATHS, ensureDir, atomicWrite, readJSONFile, sha256File } from '../../lib/fileUtils.js'; +import { PATHS, ensureDir, atomicWrite, readJSONFile, sha256File, copyFileGuarded } from '../../lib/fileUtils.js'; import { getOrComputeImageSha256 } from '../../lib/assetHash.js'; import { isPlainObject } from '../../lib/objects.js'; import { getBucket, ensureBucketLayout, bucketBlobsDir, bucketBlobPath, bucketBlobSidecarPath, bucketBlobIndexPath, bucketRecordsDir, bucketRecordPath, imageSidecarName, isHexHash } from './buckets.js'; @@ -148,12 +148,12 @@ async function copyAssetIfPresent(filename, kind, bucketPath, cache) { if (cache) cache[cacheKey] = hash; } const blobPath = bucketBlobPath(bucketPath, hash); - if (!existsSync(blobPath)) await copyFile(sourcePath, blobPath); + if (!existsSync(blobPath)) await copyFileGuarded(sourcePath, blobPath); if (kind === 'image') { const sidecarSource = join(sourceDir, imageSidecarName(base)); if (existsSync(sidecarSource)) { const sidecarTarget = bucketBlobSidecarPath(bucketPath, hash); - if (!existsSync(sidecarTarget)) await copyFile(sidecarSource, sidecarTarget); + if (!existsSync(sidecarTarget)) await copyFileGuarded(sidecarSource, sidecarTarget); } } return { kind, ref: base, hash }; diff --git a/server/services/shellImageDrop.js b/server/services/shellImageDrop.js index 665bead3ea..1033e5ee51 100644 --- a/server/services/shellImageDrop.js +++ b/server/services/shellImageDrop.js @@ -17,8 +17,7 @@ * and over, and a second drop must not overwrite bytes the agent hasn't read yet. */ -import { unlink } from 'fs/promises'; -import { PATHS, saveImageUpload } from '../lib/fileUtils.js'; +import { PATHS, saveImageUpload, unlinkGuarded } from '../lib/fileUtils.js'; import { MAX_SCREENSHOT_BYTES } from '../lib/uploadLimits.js'; import { ServerError } from '../lib/errorHandler.js'; import { v4 as uuidv4 } from '../lib/uuid.js'; @@ -76,7 +75,7 @@ export async function dropImageIntoShellSession({ sessionId, filename, data, mes // with the screenshot uploads, so an orphan here is indistinguishable from a // real one. Best-effort: a failed unlink must not mask the 404. if (!pasteToSession(sessionId, buildImageDropText(saved.filePath, message), { label: 'image drop' })) { - await unlink(saved.filePath).catch(() => {}); + await unlinkGuarded(saved.filePath).catch(() => {}); throw new ServerError('Session not found', { status: 404, code: 'NOT_FOUND' }); } diff --git a/server/services/shellImageDrop.test.js b/server/services/shellImageDrop.test.js index 866adf554f..741d95b741 100644 --- a/server/services/shellImageDrop.test.js +++ b/server/services/shellImageDrop.test.js @@ -8,6 +8,7 @@ vi.mock('./shell.js', () => ({ vi.mock('../lib/fileUtils.js', () => ({ PATHS: { screenshots: '/opt/portos/data/screenshots' }, saveImageUpload: vi.fn(), + unlinkGuarded: vi.fn().mockResolvedValue(undefined), })); import { buildImageDropText, dropImageIntoShellSession } from './shellImageDrop.js'; diff --git a/server/services/sprites/animationTrackWorkflow.js b/server/services/sprites/animationTrackWorkflow.js index 2a2e1cc067..9aeb7f8b0d 100644 --- a/server/services/sprites/animationTrackWorkflow.js +++ b/server/services/sprites/animationTrackWorkflow.js @@ -38,9 +38,9 @@ */ import { join } from 'path'; -import { readdir, rm } from 'fs/promises'; +import { readdir } from 'fs/promises'; import { randomUUID } from 'crypto'; -import { atomicWrite, ensureDir, pathExists, readJSONFile, sha256File } from '../../lib/fileUtils.js'; +import { atomicWrite, ensureDir, pathExists, readJSONFile, sha256File, rmGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { executeTuiRun } from '../tuiPromptRunner.js'; import { GROK_TUI_ID } from '../../lib/grok.js'; @@ -607,7 +607,7 @@ async function invalidateTrackDirectionImpl( const approved = loaded?.directions?.[direction] || finalizedSet?.directions?.[direction]; if (approved?.status !== 'approved') return false; const selection = loaded || { ...seedSelection(row, recordId), directions: { ...(finalizedSet?.directions || {}) } }; - if (finalizedSet) await rm(join(spriteDir(recordId), trackSetRelPath(row.id, recordId)), { force: true }); + if (finalizedSet) await rmGuarded(join(spriteDir(recordId), trackSetRelPath(row.id, recordId)), { force: true }); delete selection.directions[direction]; selection.status = 'in-progress'; await ensureDir(join(spriteDir(recordId), row.id)); diff --git a/server/services/sprites/assets.js b/server/services/sprites/assets.js index 9d2ed70ec0..1c784a5ed3 100644 --- a/server/services/sprites/assets.js +++ b/server/services/sprites/assets.js @@ -20,8 +20,7 @@ */ import { join, dirname, relative, sep } from 'path'; -import { rm } from 'fs/promises'; -import { readJSONFile } from '../../lib/fileUtils.js'; +import { readJSONFile, rmGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { spriteDir, resolveSpriteAssetPath, RUNTIME_POINTER_REL, RUNTIME_PUBLICATIONS_REL, @@ -83,7 +82,7 @@ async function deleteSpriteAssetImpl(recordId, relPath) { // double-click / already-gone file is a no-op success, not a 500. const removed = versionMatch ? versionMatch[1] : normalized; const removeAbs = versionMatch ? join(dir, versionMatch[1]) : abs; - await rm(removeAbs, { recursive: true, force: true }); + await rmGuarded(removeAbs, { recursive: true, force: true }); console.log(`🗑️ sprite asset deleted for ${recordId} → ${removed}`); return { deleted: true, removed }; } diff --git a/server/services/sprites/importer.js b/server/services/sprites/importer.js index 8031375ff7..522e85cdba 100644 --- a/server/services/sprites/importer.js +++ b/server/services/sprites/importer.js @@ -23,8 +23,8 @@ */ import { join, basename, dirname } from 'path'; -import { readdir, copyFile, rm } from 'fs/promises'; -import { PATHS, ensureDir, sha256File, atomicWrite, pathExists, readJSONFile, expandHome } from '../../lib/fileUtils.js'; +import { readdir } from 'fs/promises'; +import { PATHS, ensureDir, sha256File, atomicWrite, pathExists, readJSONFile, expandHome, copyFileGuarded, rmGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { upsertImportedRecord } from './records.js'; import { @@ -78,7 +78,7 @@ async function copyTree(srcDir, destDir, shouldCopy = () => true) { await ensureDir(parent); ensuredDirs.add(parent); } - await copyFile(srcPath, destPath); + await copyFileGuarded(srcPath, destPath); copied.push(entryRel); } } @@ -114,7 +114,7 @@ async function verifyHashes(destDir, expectations, errors, copiedThisRun) { // nobody approved. Drop it — that run just stays non-re-derivable until // a clean re-import. if (SOURCE_CLIP_REF.test(relPath)) { - await rm(join(destDir, relPath), { force: true }); + await rmGuarded(join(destDir, relPath), { force: true }); return { dropped: relPath }; } return { error: `sha256 mismatch: ${relPath}` }; @@ -237,7 +237,7 @@ async function copyCharFile(srcCharDir, destDir, rel) { const destRel = canonicalizeImportedRunPath(rel); const dest = join(destDir, destRel); await ensureDir(dirname(dest)); - await copyFile(join(srcCharDir, srcRel), dest); + await copyFileGuarded(join(srcCharDir, srcRel), dest); return destRel; } @@ -405,7 +405,7 @@ async function importCharacter({ sourceRoot, characterId, spec, specPath, select await ensureDir(destDir); // Character spec — verbatim copy for provenance alongside the record. - await copyFile(specPath, join(destDir, 'character-spec.json')); + await copyFileGuarded(specPath, join(destDir, 'character-spec.json')); result.files += 1; const hashExpectations = []; diff --git a/server/services/sprites/localAnimationRender.js b/server/services/sprites/localAnimationRender.js index dab3c0b96a..faa694d27f 100644 --- a/server/services/sprites/localAnimationRender.js +++ b/server/services/sprites/localAnimationRender.js @@ -27,8 +27,7 @@ */ import { join } from 'path'; -import { copyFile } from 'fs/promises'; -import { PATHS, pathExists } from '../../lib/fileUtils.js'; +import { PATHS, pathExists, copyFileGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { getVideoModels, requiredModelCacheGroups } from '../../lib/mediaModels.js'; import { inspectModelCache, findCachedRepoFiles } from '../../lib/hfCache.js'; @@ -380,7 +379,7 @@ export async function collectLocalAnimationClip({ jobId, videoAbs, label }) { return false; } try { - await copyFile(renderedAbs, videoAbs); + await copyFileGuarded(renderedAbs, videoAbs); return true; } catch (err) { console.error(`❌ ${label} could not stage the local clip into its run: ${err?.message || err}`); diff --git a/server/services/sprites/normalize.js b/server/services/sprites/normalize.js index b837ebf0af..d8403a9933 100644 --- a/server/services/sprites/normalize.js +++ b/server/services/sprites/normalize.js @@ -28,8 +28,7 @@ */ import sharp from 'sharp'; -import { copyFile } from 'fs/promises'; -import { tryReadFile } from '../../lib/fileUtils.js'; +import { tryReadFile, copyFileGuarded } from '../../lib/fileUtils.js'; import { hexToRgb, keyChannelSplit, keyShareFn } from './chromaKey.js'; import { describeFrameStats, isDegenerateFrame } from '../../lib/imageFrameStats.js'; import { ServerError } from '../../lib/errorHandler.js'; @@ -203,7 +202,7 @@ export async function extractForegroundPalette(src, maskKeyHex) { export async function normalizeFromAnalysis(analysis, src, dest, canvasKeyHex) { const { data, width, mask, bbox, maskKey } = analysis; if (!bbox) { - await copyFile(src, dest); + await copyFileGuarded(src, dest); return { copiedThrough: true }; } const charW = bbox.right - bbox.left; @@ -253,7 +252,7 @@ export async function normalizeFromAnalysis(analysis, src, dest, canvasKeyHex) { export async function recompositeOnKey(analysis, src, dest, canvasKeyHex) { const { data, width, height, mask, bbox, maskKey } = analysis; if (!bbox) { - await copyFile(src, dest); + await copyFileGuarded(src, dest); return { copiedThrough: true }; } const fill = hexToRgb(canvasKeyHex); diff --git a/server/services/sprites/reference.js b/server/services/sprites/reference.js index 108fa861ce..b9264de2ea 100644 --- a/server/services/sprites/reference.js +++ b/server/services/sprites/reference.js @@ -23,10 +23,10 @@ */ import { join } from 'path'; -import { readdir, copyFile } from 'fs/promises'; +import { readdir } from 'fs/promises'; import { PATHS, ensureDir, sha256File, atomicWrite, pathExists, readJSONFile, - importFileToDir, listDirectoryByExtension, resolveGalleryImage, + importFileToDir, listDirectoryByExtension, resolveGalleryImage, copyFileGuarded, } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { createKeyCachedQueue } from '../../lib/createKeyCachedQueue.js'; @@ -804,7 +804,7 @@ export async function attachReferenceCandidate(ctx) { await ensureDir(candidatesDir); const name = await nextCandidateName(candidatesDir, anchorId); const dest = join(candidatesDir, name); - await copyFile(src, dest); + await copyFileGuarded(src, dest); const relPath = `reference/candidates/${name}`; await atomicWrite(join(candidatesDir, `${name.replace(/\.png$/, '')}.generation.json`), { schemaVersion: 1, diff --git a/server/services/sprites/walk.js b/server/services/sprites/walk.js index 6f9aab3c4d..6d101320bf 100644 --- a/server/services/sprites/walk.js +++ b/server/services/sprites/walk.js @@ -19,10 +19,10 @@ */ import { join } from 'path'; -import { readdir, rm, stat } from 'fs/promises'; +import { readdir, stat } from 'fs/promises'; import { randomUUID } from 'crypto'; import { - ensureDir, atomicWrite, readJSONFile, pathExists, sha256File, + ensureDir, atomicWrite, readJSONFile, pathExists, sha256File, rmGuarded, } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { executeTuiRun } from '../tuiPromptRunner.js'; @@ -1843,7 +1843,7 @@ async function assertDirectionReDerivable(recordId, walkState, direction, { ackn // walk-complete record with no frozen set behind it. Shared by unlock (re-opens // all directions) and reopen (re-opens one). async function dropFinalizedWalkSet(recordId, recordStatus = 'reference-complete') { - await rm(join(spriteDir(recordId), walkSetRelPath(recordId)), { force: true }); + await rmGuarded(join(spriteDir(recordId), walkSetRelPath(recordId)), { force: true }); await updateRecord(recordId, { status: recordStatus }); } diff --git a/server/services/sprites/walkTrims.js b/server/services/sprites/walkTrims.js index 3018f4a72b..31044cbfb2 100644 --- a/server/services/sprites/walkTrims.js +++ b/server/services/sprites/walkTrims.js @@ -15,10 +15,10 @@ */ import { join } from 'path'; -import { mkdtemp, rm, writeFile } from 'fs/promises'; +import { mkdtemp } from 'fs/promises'; import { tmpdir } from 'os'; import { - ensureDir, atomicWrite, pathExists, sha256File, readJSONFile, + ensureDir, atomicWrite, pathExists, sha256File, readJSONFile, rmGuarded, writeFileGuarded, } from '../../lib/fileUtils.js'; import { findFfmpeg, runFfmpegProcess } from '../../lib/ffmpeg.js'; import { ServerError } from '../../lib/errorHandler.js'; @@ -49,7 +49,7 @@ async function encodeTrimGif(frames, fps, destAbs) { if (!ffmpeg) throw new ServerError('ffmpeg not found — install ffmpeg to render trim GIFs', { status: 503, code: 'FFMPEG_MISSING' }); const scratch = await mkdtemp(join(tmpdir(), 'portos-sprite-trim-')); for (let i = 0; i < frames.length; i++) { - await writeFile( + await writeFileGuarded( join(scratch, `frame-${String(i).padStart(3, '0')}.png`), await encodePng(frames[i]), ); @@ -64,7 +64,7 @@ async function encodeTrimGif(frames, fps, destAbs) { destAbs, ], }); - await rm(scratch, { recursive: true, force: true }).catch(() => {}); + await rmGuarded(scratch, { recursive: true, force: true }).catch(() => {}); if (!result.ok) throw new ServerError(`GIF encode failed: ${result.reason}`, { status: 500, code: 'GIF_ENCODE_FAILED' }); } @@ -197,7 +197,7 @@ async function saveLoopTrimImpl(recordId, payload) { width: cellSize * frames.length, height: cellSize, }); - await writeFile(join(trimsAbs, stripName), stripBuf); + await atomicWrite(join(trimsAbs, stripName), stripBuf); const gifName = `${prefix}.gif`; await encodeTrimGif(frames, fps, join(trimsAbs, gifName)); diff --git a/server/services/subAgentSpawner.js b/server/services/subAgentSpawner.js index 46c7bc0d31..57eef64538 100644 --- a/server/services/subAgentSpawner.js +++ b/server/services/subAgentSpawner.js @@ -29,13 +29,13 @@ */ import { join } from 'path'; -import { readdir, rm, stat } from 'fs/promises'; +import { readdir, stat } from 'fs/promises'; import { existsSync } from 'fs'; import { emitLog, cosEvents } from './cosEvents.js'; import { updateAgent } from './cosAgentLifecycle.js'; import { initProviderStatus } from './providerStatus.js'; import { onCosRunnerEvent, initCosRunnerConnection, isRunnerAvailable, isRunnerReachable } from './cosRunnerClient.js'; -import { PATHS } from '../lib/fileUtils.js'; +import { PATHS, rmGuarded } from '../lib/fileUtils.js'; import { loadSlashdoFile } from '../lib/slashdoLoader.js'; import { getRunnerOutputBatcher, flushRunnerOutputBatcher } from './agentRunnerOutputBatchers.js'; import { syncRunnerAgents } from './agentRunnerSync.js'; @@ -269,7 +269,7 @@ async function runInitSpawner() { const runDir = join(RUNS_DIR, entry.name); const dirStat = await stat(runDir).catch(() => null); if (dirStat && dirStat.mtime.getTime() < cutoff) { - await rm(runDir, { recursive: true }).catch(() => {}); + await rmGuarded(runDir, { recursive: true }).catch(() => {}); pruned++; } } diff --git a/server/services/supersededBackup.js b/server/services/supersededBackup.js index 8cf92a8442..b2a597b084 100644 --- a/server/services/supersededBackup.js +++ b/server/services/supersededBackup.js @@ -24,9 +24,9 @@ */ import { createHash } from 'node:crypto'; -import { cp, stat, writeFile } from 'node:fs/promises'; +import { stat } from 'node:fs/promises'; import { dirname, join } from 'node:path'; -import { atomicWrite, ensureDir, PATHS } from '../lib/fileUtils.js'; +import { atomicWrite, ensureDir, PATHS, writeFileGuarded, copyFileGuarded } from '../lib/fileUtils.js'; import { isAgentScratchPath } from '../lib/agentScratchPaths.js'; import { kebabCase } from '../lib/textUtils.js'; import { execGitSafe } from './git.js'; @@ -112,7 +112,7 @@ async function copyUntracked(worktreePath, destDir) { await ensureDir(parent); created.add(parent); } - const ok = await cp(source, destination).then(() => true, () => false); + const ok = await copyFileGuarded(source, destination).then(() => true, () => false); if (ok) { copied.push(rel); total += size; @@ -141,7 +141,7 @@ export async function backupSupersededBranch(repoPath, branch, { defaultBranch = const commitsPath = join(dir, 'commits.patch'); const commits = await gitText(['format-patch', `${defaultBranch}..${branch.branch}`, '--stdout'], repoPath); const hasCommits = Boolean(commits.trim()); - if (hasCommits) await writeFile(commitsPath, commits); + if (hasCommits) await writeFileGuarded(commitsPath, commits); const diffPath = join(dir, 'worktree.diff'); let hasDiff = false; @@ -149,7 +149,7 @@ export async function backupSupersededBranch(repoPath, branch, { defaultBranch = if (branch.worktreePath) { const worktreeDiff = await gitText(['diff', 'HEAD', '--binary'], branch.worktreePath); hasDiff = Boolean(worktreeDiff.trim()); - if (hasDiff) await writeFile(diffPath, worktreeDiff); + if (hasDiff) await writeFileGuarded(diffPath, worktreeDiff); // No upfront mkdir: copyUntracked creates each parent lazily, so a worktree // with nothing untracked leaves no empty directory behind. untracked = await copyUntracked(branch.worktreePath, join(dir, 'untracked')); diff --git a/server/services/syncOrchestrator.js b/server/services/syncOrchestrator.js index e14b5b74bb..62004fd5ec 100644 --- a/server/services/syncOrchestrator.js +++ b/server/services/syncOrchestrator.js @@ -6,9 +6,9 @@ * Maintains per-peer cursors and triggers sync on peer connect + interval. */ -import { writeFile, access } from 'fs/promises'; +import { access } from 'fs/promises'; import { join } from 'path'; -import { readJSONFile, ensureDir, PATHS, dataPath, atomicWrite } from '../lib/fileUtils.js'; +import { readJSONFile, ensureDir, PATHS, dataPath, atomicWrite, writeFileGuarded } from '../lib/fileUtils.js'; import { createMutex } from '../lib/asyncMutex.js'; import { instanceEvents } from './instanceEvents.js'; import { getPeers, resolveEffectiveCategories, updatePeer, getInstanceId, UNKNOWN_INSTANCE_ID } from './instances.js'; @@ -122,7 +122,7 @@ async function syncImageFromPeer(peer, avatarPath) { await res.arrayBuffer() .then(async (bytes) => { await ensureDir(PATHS.images); - await writeFile(localPath, Buffer.from(bytes)); + await writeFileGuarded(localPath, Buffer.from(bytes)); console.log(`🔄 Synced avatar image: ${filename}`); }) .catch(() => {}); diff --git a/server/services/taste-questionnaire.js b/server/services/taste-questionnaire.js index b617e97f1c..89e80216f5 100644 --- a/server/services/taste-questionnaire.js +++ b/server/services/taste-questionnaire.js @@ -6,7 +6,7 @@ * with branching follow-up questions based on responses. */ -import { readFile, writeFile } from 'fs/promises'; +import { readFile } from 'fs/promises'; import { existsSync } from 'fs'; import { join } from 'path'; import { v4 as uuidv4 } from '../lib/uuid.js'; @@ -1039,7 +1039,7 @@ async function appendToAestheticsDoc(sectionId, config, questionId, answer) { content += entry; } - await writeFile(targetPath, content); + await atomicWrite(targetPath, content); } async function writeAestheticsDocument(profileSummary, completedSections) { @@ -1062,5 +1062,5 @@ async function writeAestheticsDocument(profileSummary, completedSections) { } } - await writeFile(targetPath, content); + await atomicWrite(targetPath, content); } diff --git a/server/services/universeCharacterSheet.js b/server/services/universeCharacterSheet.js index f729fb9467..bb905914da 100644 --- a/server/services/universeCharacterSheet.js +++ b/server/services/universeCharacterSheet.js @@ -13,9 +13,8 @@ * `character.referenceSheetImageRef` once the render completes. */ -import { copyFile, unlink } from 'fs/promises'; import { join, basename } from 'path'; -import { PATHS, ensureDir, shortId, assertSafeFilename } from '../lib/fileUtils.js'; +import { PATHS, ensureDir, shortId, assertSafeFilename, copyFileGuarded, unlinkGuarded } from '../lib/fileUtils.js'; import { ServerError } from '../lib/errorHandler.js'; import { getSettings } from './settings.js'; import { getUniverse, updateUniverse } from './universeBuilder.js'; @@ -588,7 +587,7 @@ export async function onSheetComplete({ universeId, entryId, jobId, sourceFilena // ALWAYS copy the file — even superseded renders are kept on disk for // rollback/comparison (they live at `data/image-refs/<...>--.png` // with a unique per-job filename). - await copyFile(srcPath, destPath); + await copyFileGuarded(srcPath, destPath); console.log(`📸 ${variantConfig.label} copied to image-refs: ${destFilename}`); // If a newer render has been started for this character+variant while ours @@ -679,7 +678,7 @@ export async function deleteCharacterReferenceSheet(universeId, entryId, { varia // cleanup, sample-data reset). The pointer-purge below is the canonical // clean and runs regardless. let fileDeleted = true; - await unlink(target).catch((err) => { + await unlinkGuarded(target).catch((err) => { if (err?.code === 'ENOENT') { fileDeleted = false; return; } throw err; }); diff --git a/server/services/universeCharacterSheetDelete.test.js b/server/services/universeCharacterSheetDelete.test.js index c8c75cfb85..20e0a51087 100644 --- a/server/services/universeCharacterSheetDelete.test.js +++ b/server/services/universeCharacterSheetDelete.test.js @@ -37,6 +37,8 @@ vi.mock('../lib/fileUtils.js', () => ({ PATHS: { imageRefs: '/mock/data/image-refs', images: '/mock/data/images', data: '/mock/data' }, ensureDir: vi.fn().mockResolvedValue(undefined), shortId: (id, n = 8) => (id == null ? '' : String(id).slice(0, n)), + unlinkGuarded: (...args) => unlinkMock(...args), + copyFileGuarded: vi.fn(), // Real basename-only validator behavior the helper relies on as defense-in-depth. assertSafeFilename: vi.fn((filename) => { if (!filename || typeof filename !== 'string') { diff --git a/server/services/videoGen/chainedVideo.js b/server/services/videoGen/chainedVideo.js index 34e6937165..1cf35320d2 100644 --- a/server/services/videoGen/chainedVideo.js +++ b/server/services/videoGen/chainedVideo.js @@ -1,10 +1,9 @@ /** Multi-chunk local-video orchestration. */ -import { unlink } from 'fs/promises'; import { join } from 'path'; import { randomUUID } from 'crypto'; import { tmpdir } from 'os'; -import { PATHS } from '../../lib/fileUtils.js'; +import { PATHS, unlinkGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { probeFrameCount, trimVideoFromFrame } from '../../lib/ffmpeg.js'; import { @@ -300,7 +299,7 @@ export async function generateChainedVideo({ chunks, chunkPrompts, contextFrames // long chain doesn't leave a trail of clips in tmpdir. Best-effort and // fire-and-forget: a leftover temp file must never fail a finished render. const cleanupContextClips = () => { - for (const p of contextClipPaths) unlink(p).catch(() => {}); + for (const p of contextClipPaths) unlinkGuarded(p).catch(() => {}); contextClipPaths.length = 0; }; const finishOk = (payload) => { diff --git a/server/services/videoGen/deleteHistoryItem.test.js b/server/services/videoGen/deleteHistoryItem.test.js index a6189fe321..bea4e31ea3 100644 --- a/server/services/videoGen/deleteHistoryItem.test.js +++ b/server/services/videoGen/deleteHistoryItem.test.js @@ -30,6 +30,7 @@ vi.mock('../../lib/fileUtils.js', () => ({ readJSONFile: vi.fn(async () => []), atomicWrite: vi.fn(async () => {}), assertSafeFilename: vi.fn(), + unlinkGuarded: (...args) => unlink(...args), UUID_RE: /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i, })); diff --git a/server/services/videoGen/frameExtraction.js b/server/services/videoGen/frameExtraction.js index 1d4af73373..8022f2b05e 100644 --- a/server/services/videoGen/frameExtraction.js +++ b/server/services/videoGen/frameExtraction.js @@ -1,11 +1,10 @@ /** Video frame extraction and evaluation-frame sampling. */ import { existsSync, statSync } from 'fs'; -import { unlink, writeFile, copyFile, rm } from 'fs/promises'; import { join } from 'path'; import { tmpdir } from 'os'; import { spawn } from '../../lib/childProcess.js'; -import { ensureDir, PATHS, UUID_RE, tryReadFile } from '../../lib/fileUtils.js'; +import { ensureDir, PATHS, UUID_RE, tryReadFile, copyFileGuarded, writeFileGuarded, unlinkGuarded, rmGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { findFfmpeg, safeUnder, extractEvaluationFrames, runFfmpegProcess, @@ -32,7 +31,7 @@ async function decodeTailCandidates(ffmpeg, videoPath, candidateDir) { // Clear first: a crashed prior run can leave a longer numbered run behind, // and the by-name enumeration below would read those stale frames as this // clip's candidates. - await rm(candidateDir, { recursive: true, force: true }).catch(() => {}); + await rmGuarded(candidateDir, { recursive: true, force: true }).catch(() => {}); await ensureDir(candidateDir); const outPattern = join(candidateDir, 'cand-%03d.png'); // Through the catalog helper rather than a hand-rolled spawn: it already @@ -158,7 +157,7 @@ export async function extractLastFrame(historyId) { kind: 'extracted-frame', createdAt: new Date().toISOString(), }; - await writeFile(sidecarPath, JSON.stringify(meta, null, 2), { flag: 'wx' }).catch(() => {}); + await writeFileGuarded(sidecarPath, JSON.stringify(meta, null, 2), { flag: 'wx' }).catch(() => {}); }; // A cached anchor the fallback produced WITHOUT a scan is provisional: serving @@ -179,7 +178,7 @@ export async function extractLastFrame(historyId) { await writeSidecar(); return { filename: frameFilename, path: `/data/images/${frameFilename}` }; } - if (cachedSize === 0) await unlink(framePath).catch(() => {}); + if (cachedSize === 0) await unlinkGuarded(framePath).catch(() => {}); // ── Scored pick over the tail window ────────────────────────────────────── // Everything here degrades to the single-seek fallback below rather than @@ -205,7 +204,7 @@ export async function extractLastFrame(historyId) { // ffmpeg write below has always had, not a new class.) const expectedSize = best ? safeStatSize(best.path) : null; const installed = best - ? await copyFile(best.path, framePath).then(() => { + ? await copyFileGuarded(best.path, framePath).then(() => { const written = safeStatSize(framePath); if (written && (expectedSize == null || written === expectedSize)) return true; console.log(`⚠️ Anchor install wrote ${written ?? 'nothing'} of ${expectedSize ?? '?'} bytes — discarding`); @@ -215,18 +214,18 @@ export async function extractLastFrame(historyId) { return false; }) : false; - if (best && !installed) await unlink(framePath).catch(() => {}); + if (best && !installed) await unlinkGuarded(framePath).catch(() => {}); // Drop the whole candidate dir either way — they're temp decodes and the // winner is already copied into data/images/ by now. `item.id` is validated // UUID/`upload-` above, so the recursive remove can't escape tmpdir. - await rm(candidateDir, { recursive: true, force: true }).catch(() => {}); + await rmGuarded(candidateDir, { recursive: true, force: true }).catch(() => {}); // Both extraction paths below know the truth about this anchor, so they must // be able to REPLACE a sidecar written by an earlier attempt — `wx` alone // would leave a stale provisional marker in place and re-scan forever. This // is the extraction path (the cache hit already returned), so an unconditional // drop is right: whatever is written next is authoritative. - await unlink(sidecarPath).catch(() => {}); + await unlinkGuarded(sidecarPath).catch(() => {}); if (installed) { // Offset derived from the candidates actually decoded, not from a nominal @@ -272,7 +271,7 @@ export async function extractLastFrame(historyId) { // A 0-byte file is a partial extraction, not a cache-worthy result — // delete it so the next call retries instead of returning a broken // image from the cache hit above. - if (writtenSize === 0) await unlink(framePath).catch(() => {}); + if (writtenSize === 0) await unlinkGuarded(framePath).catch(() => {}); return reject(new ServerError('Failed to extract last frame', { status: 500, code: 'FFMPEG_FAILED' })); } // A scan that RAN and found nothing usable is a property of the clip — diff --git a/server/services/videoGen/generateVideo.js b/server/services/videoGen/generateVideo.js index 3bdcdc4a37..77015a5502 100644 --- a/server/services/videoGen/generateVideo.js +++ b/server/services/videoGen/generateVideo.js @@ -11,12 +11,12 @@ */ import { execFile } from '../../lib/childProcess.js'; -import { unlink, rm, mkdtemp } from 'fs/promises'; +import { mkdtemp } from 'fs/promises'; import { join, basename } from 'path'; import { tmpdir } from 'os'; import { randomUUID } from 'crypto'; import { promisify } from 'util'; -import { ensureDir, PATHS } from '../../lib/fileUtils.js'; +import { ensureDir, PATHS, unlinkGuarded, rmGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { isDefaultI2vReferenceMode, normalizeI2vReferenceMode, resolveI2vReferenceStrength, @@ -670,7 +670,7 @@ export async function generateVideo({ pythonPath, prompt, negativePrompt = '', m ? [audioFilePath] : []), ]; - return Promise.all(paths.filter(Boolean).map((path) => unlink(path).catch(() => {}))); + return Promise.all(paths.filter(Boolean).map((path) => unlinkGuarded(path).catch(() => {}))); }; if (isIcLoraMode(mode) && icLoraSpecForMode(mode)?.referenceKind === 'image' && Array.isArray(icReferencePaths) && icReferencePaths.length) { @@ -899,7 +899,7 @@ export async function generateVideo({ pythonPath, prompt, negativePrompt = '', m broadcastSse(job, { type: 'error', error: reason }); videoGenEvents.emit('failed', { generationId: jobId, error: reason }); void cleanupTempFiles({ includeUploads: true, includeUntrackedAudio: true }); - void rm(stepwiseDir, { recursive: true, force: true }); + void rmGuarded(stepwiseDir, { recursive: true, force: true }); closeJobAfterDelay(videoJobState.jobs, jobId); throw err; } diff --git a/server/services/videoGen/grok.js b/server/services/videoGen/grok.js index 75055b14fd..2dc1a69b24 100644 --- a/server/services/videoGen/grok.js +++ b/server/services/videoGen/grok.js @@ -22,11 +22,11 @@ */ import { spawn } from '../../lib/childProcess.js'; -import { copyFile, mkdir, open, rename, rm, stat, unlink } from 'fs/promises'; +import { mkdir, open, stat } from 'fs/promises'; import { isAbsolute, join, resolve as pathResolve, sep } from 'path'; import { tmpdir } from 'os'; import { randomUUID } from 'crypto'; -import { ensureDir, PATHS } from '../../lib/fileUtils.js'; +import { ensureDir, PATHS, copyFileGuarded, unlinkGuarded, rmGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { broadcastSse, attachSseClient as attachSse, closeJobAfterDelay } from '../../lib/sseUtils.js'; import { killWithEscalation } from '../../lib/killWithEscalation.js'; @@ -209,12 +209,12 @@ async function runGrokVideo(job, jobId, bin, args, { // scratch-dir spawn telling the child one consistent story about where it is. const proc = spawn(spawnBin, spawnArgs, { cwd: scratchDir, env: withSpawnCwdEnv(process.env, scratchDir), shell: false, stdio: [useStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'] }); activeProcs.set(jobId, proc); - const removeScratch = () => rm(scratchDir, { recursive: true, force: true }).catch(() => {}); + const removeScratch = () => rmGuarded(scratchDir, { recursive: true, force: true }).catch(() => {}); // The route stages a multipart source image into data/uploads and hands us // its path — the provider owns unlinking it on every terminal path (same // contract as videoGen/local.js; the queue only cleans up when the // provider throws before spawning, or on boot-restore of a dead job). - const removeUpload = () => { if (uploadedTempPath) unlink(uploadedTempPath).catch(() => {}); }; + const removeUpload = () => { if (uploadedTempPath) unlinkGuarded(uploadedTempPath).catch(() => {}); }; if (useStdin) { proc.stdin.on('error', () => {}); @@ -276,12 +276,8 @@ async function runGrokVideo(job, jobId, bin, args, { const prefix = harvested.invalid ? 'Grok wrote a non-MP4 file at the directed path. ' : ''; return finalizeError(job, jobId, proc, `${prefix}${noVideoReason(stdoutTail)}`); } - // Move, not copy (metadata-only when tmp and the videos dir share a - // filesystem); copy+unlink is the cross-device fallback. - await rename(stagingPath, outputPath).catch(async () => { - await copyFile(stagingPath, outputPath); - await unlink(stagingPath).catch(() => {}); - }); + await copyFileGuarded(stagingPath, outputPath); + await unlinkGuarded(stagingPath).catch(() => {}); removeScratch(); removeUpload(); if (activeProcs.get(jobId) === proc) activeProcs.delete(jobId); diff --git a/server/services/videoGen/historyOps.js b/server/services/videoGen/historyOps.js index c91c7752c6..c4d0b696f1 100644 --- a/server/services/videoGen/historyOps.js +++ b/server/services/videoGen/historyOps.js @@ -1,7 +1,6 @@ /** User-facing video-history mutations. */ -import { unlink } from 'fs/promises'; -import { PATHS } from '../../lib/fileUtils.js'; +import { PATHS, unlinkGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { safeUnder } from '../../lib/ffmpeg.js'; import { loadHistory, mutateVideoHistory } from './history.js'; @@ -47,17 +46,17 @@ export async function deleteHistoryItem(id) { // Same path-traversal guard as extractLastFrame — unlink only if the // filename resolves to inside the expected dir. const videoFile = safeUnder(PATHS.videos, item.filename); - if (videoFile) await unlink(videoFile).catch(() => {}); + if (videoFile) await unlinkGuarded(videoFile).catch(() => {}); if (item.thumbnail) { const thumbFile = safeUnder(PATHS.videoThumbnails, item.thumbnail); - if (thumbFile) await unlink(thumbFile).catch(() => {}); + if (thumbFile) await unlinkGuarded(thumbFile).catch(() => {}); } // Delete evaluation frame thumbnails written by sampleEvaluationFrames: // `${jobId}-f1.jpg` … `${jobId}-f9.jpg` (max count in sampleEvaluationFrames is 5, // but 9 is a safe upper bound to catch any future increase). for (let i = 1; i <= 9; i++) { const frameFile = safeUnder(PATHS.videoThumbnails, `${id}-f${i}.jpg`); - if (frameFile) await unlink(frameFile).catch(() => {}); + if (frameFile) await unlinkGuarded(frameFile).catch(() => {}); } // Serialized removal through the shared tail (re-filters the freshest list), // so a concurrent download/render append isn't dropped by this save. diff --git a/server/services/videoGen/local.test.js b/server/services/videoGen/local.test.js index 65213fcf23..cbb80d96ee 100644 --- a/server/services/videoGen/local.test.js +++ b/server/services/videoGen/local.test.js @@ -69,12 +69,26 @@ async function waitForStitch() { } } +const fsPromisesMock = vi.hoisted(() => ({ + unlink: vi.fn(async () => {}), + writeFile: vi.fn(async () => {}), + copyFile: vi.fn(async () => {}), + rm: vi.fn(async () => {}), + readFile: vi.fn(async () => Buffer.from('')), + mkdtemp: vi.fn(async (prefix) => `${prefix}mock`), + rename: vi.fn(async () => {}), +})); + vi.mock('../../lib/fileUtils.js', () => ({ -tryReadFile: vi.fn().mockResolvedValue(null), + tryReadFile: vi.fn().mockResolvedValue(null), ensureDir: vi.fn(async () => {}), PATHS: MOCK_PATHS, readJSONFile: vi.fn(async () => []), atomicWrite: vi.fn(async () => {}), + copyFileGuarded: fsPromisesMock.copyFile, + writeFileGuarded: fsPromisesMock.writeFile, + unlinkGuarded: fsPromisesMock.unlink, + rmGuarded: fsPromisesMock.rm, // resolveVideoLoras → assertSafeLoraFilename → assertSafeFilename; the // filename safety check is unit-tested in loras.test.js, so a no-op here // lets the LoRA-arg test focus on the spawn-args plumbing. @@ -385,17 +399,7 @@ vi.mock('../loraEffectProbe.js', () => ({ probeLoraEffect: vi.fn(async (filename) => loraEffectState.reportByFilename[filename] || loraEffectState.defaultReport), })); -vi.mock('fs/promises', () => ({ - unlink: vi.fn(async () => {}), - writeFile: vi.fn(async () => {}), - copyFile: vi.fn(async () => {}), - rm: vi.fn(async () => {}), - readFile: vi.fn(async () => Buffer.from('')), - mkdtemp: vi.fn(async (prefix) => `${prefix}mock`), - // Unused by the code under test, but lib/ffmpeg.js imports it and the ffmpeg - // mock above pulls the real module in for buildTrimConcatArgs. - rename: vi.fn(async () => {}), -})); +vi.mock('fs/promises', () => fsPromisesMock); // Fake EventEmitter-like process that completes immediately with exit code 0. // Shared shape for both the child_process spawn mock (ffmpeg/probe) and the diff --git a/server/services/videoGen/prepareParams.js b/server/services/videoGen/prepareParams.js index c0c0b2b428..6559813578 100644 --- a/server/services/videoGen/prepareParams.js +++ b/server/services/videoGen/prepareParams.js @@ -26,11 +26,10 @@ */ import { existsSync } from 'fs'; -import { copyFile, unlink } from 'fs/promises'; import { randomUUID } from 'crypto'; import { join, extname } from 'path'; import { ServerError } from '../../lib/errorHandler.js'; -import { PATHS, ensureDir, resolveGalleryImage } from '../../lib/fileUtils.js'; +import { PATHS, ensureDir, resolveGalleryImage, copyFileGuarded, unlinkGuarded } from '../../lib/fileUtils.js'; import { probeVideoDuration, safeUnder } from '../../lib/ffmpeg.js'; import { RENDER_TARGET } from '../../lib/renderTargets.js'; import { @@ -196,7 +195,7 @@ export const cleanupMultipartTemp = async (uploads) => { cleanedMultipartUploads.add(uploads); } for (const f of Object.values(uploads || {})) { - if (f?.path) await unlink(f.path).catch(() => {}); + if (f?.path) await unlinkGuarded(f.path).catch(() => {}); } }; @@ -366,7 +365,7 @@ export async function prepareVideoGenParams({ body, uploads, localOnlyParamKeys // worker's cleanup never runs). const stagedDurablePaths = []; const cleanupStaged = async () => { - for (const p of stagedDurablePaths) await unlink(p).catch(() => {}); + for (const p of stagedDurablePaths) await unlinkGuarded(p).catch(() => {}); await cleanupMultipartTemp(uploads); }; @@ -402,16 +401,16 @@ async function resolvePreparedParams({ const ext = extname(file.originalname || file.path) || '.bin'; const durablePath = join(PATHS.uploads, `video-${kind}-${randomUUID()}${ext}`); try { - await copyFile(file.path, durablePath); + await copyFileGuarded(file.path, durablePath); } catch (err) { - await unlink(durablePath).catch(() => {}); + await unlinkGuarded(durablePath).catch(() => {}); await cleanupStaged(); throw new ServerError( `Failed to stage upload to durable location: ${err.message}`, { status: 500, code: 'VIDEO_GEN_UPLOAD_STAGE_FAILED' }, ); } - await unlink(file.path).catch(() => {}); + await unlinkGuarded(file.path).catch(() => {}); stagedDurablePaths.push(durablePath); return durablePath; }; @@ -423,8 +422,8 @@ async function resolvePreparedParams({ const stageExistingAudioDurable = async (sourcePath) => { const ext = extname(sourcePath) || '.bin'; const durablePath = join(PATHS.uploads, `video-audio-${randomUUID()}${ext}`); - await copyFile(sourcePath, durablePath).catch(async (err) => { - await unlink(durablePath).catch(() => {}); + await copyFileGuarded(sourcePath, durablePath).catch(async (err) => { + await unlinkGuarded(durablePath).catch(() => {}); await cleanupStaged(); throw new ServerError( `Failed to stage project audio: ${err.message}`, @@ -728,11 +727,11 @@ async function resolvePreparedParams({ } const discardSourceImage = async () => { if (uploadedTempPath) { - await unlink(uploadedTempPath).catch(() => {}); + await unlinkGuarded(uploadedTempPath).catch(() => {}); const index = stagedDurablePaths.indexOf(uploadedTempPath); if (index >= 0) stagedDurablePaths.splice(index, 1); } - if (uploads.sourceImage?.path) await unlink(uploads.sourceImage.path).catch(() => {}); + if (uploads.sourceImage?.path) await unlinkGuarded(uploads.sourceImage.path).catch(() => {}); }; // Grok backend short-circuit (#2859 phase 2): everything past this point — // last-frame/keyframe staging, extend resolution, LoRA gating — is diff --git a/server/services/videoGen/prepareParams.test.js b/server/services/videoGen/prepareParams.test.js index be013a4fa6..4387d9eefd 100644 --- a/server/services/videoGen/prepareParams.test.js +++ b/server/services/videoGen/prepareParams.test.js @@ -29,6 +29,11 @@ vi.mock('./local.js', () => ({ DEFAULT_NUM_FRAMES: 121, })); +const { unlinkMock, copyFileMock } = vi.hoisted(() => ({ + unlinkMock: vi.fn(async () => {}), + copyFileMock: vi.fn(async () => {}), +})); + vi.mock('../../lib/fileUtils.js', () => ({ PATHS: { root: '/mock', @@ -45,12 +50,14 @@ vi.mock('../../lib/fileUtils.js', () => ({ if (!safe || safe === '.' || safe === '..') return null; return `/mock/images/${safe}`; }), + unlinkGuarded: unlinkMock, + copyFileGuarded: copyFileMock, })); vi.mock('fs', () => ({ existsSync: vi.fn(() => true) })); vi.mock('fs/promises', () => ({ - unlink: vi.fn(async () => {}), - copyFile: vi.fn(async () => {}), + unlink: unlinkMock, + copyFile: copyFileMock, })); import { unlink } from 'fs/promises'; diff --git a/server/services/videoGen/spawnWatch.js b/server/services/videoGen/spawnWatch.js index fdc024e829..9c524952fb 100644 --- a/server/services/videoGen/spawnWatch.js +++ b/server/services/videoGen/spawnWatch.js @@ -2,9 +2,9 @@ import { spawn } from '../../lib/childProcess.js'; import { watch as fsWatch } from 'fs'; -import { rm, readFile } from 'fs/promises'; +import { readFile } from 'fs/promises'; import { join } from 'path'; -import { PATHS } from '../../lib/fileUtils.js'; +import { PATHS, rmGuarded } from '../../lib/fileUtils.js'; import { spawnDetached } from '../../lib/detachedSpawn.js'; import { createLineReader } from '../../lib/streamLines.js'; import { claimHeavyLocalJob } from '../../lib/heavyJobClaim.js'; @@ -71,7 +71,7 @@ export async function spawnAndWatchVideo({ if (!heavyClaim.ok) { videoJobState.jobs.delete(jobId); await cleanupTempFiles({ includeUploads: true }); - await rm(stepwiseDir, { recursive: true, force: true }); + await rmGuarded(stepwiseDir, { recursive: true, force: true }); throw new ServerError(heavyClaim.message, { status: 409, code: 'HEAVY_LOCAL_JOB_BUSY', context: { holder: heavyClaim.holder } }); } const releaseHeavyClaim = () => heavyClaim.release() @@ -104,7 +104,7 @@ export async function spawnAndWatchVideo({ try { previewWatcher.close(); } catch { /* already closed */ } previewWatcher = null; } - void rm(stepwiseDir, { recursive: true, force: true }); + void rmGuarded(stepwiseDir, { recursive: true, force: true }); }; const processLatestPreview = async () => { if (previewClosed) return; diff --git a/server/services/videoGen/stitchVideos.js b/server/services/videoGen/stitchVideos.js index 96c1d5a5bc..a9c8fbc5e8 100644 --- a/server/services/videoGen/stitchVideos.js +++ b/server/services/videoGen/stitchVideos.js @@ -1,12 +1,11 @@ /** Lossless hand stitching and trim-aware chained-video assembly. */ import { existsSync } from 'fs'; -import { unlink, writeFile } from 'fs/promises'; import { join, basename } from 'path'; import { tmpdir } from 'os'; import { randomUUID } from 'crypto'; import { spawn } from '../../lib/childProcess.js'; -import { PATHS } from '../../lib/fileUtils.js'; +import { PATHS, writeFileGuarded, unlinkGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { findFfmpeg, safeUnder, generateThumbnail, optimizeForStreaming, @@ -116,7 +115,7 @@ export async function stitchVideos(videoIds, opts = {}) { // (which ffmpeg accepts on Windows just fine) before quoting. const escapeForConcat = (p) => p.replace(/\\/g, '/').replace(/'/g, "'\\''"); listFileWritten = true; - await writeFile(listFile, videoPaths.map((p) => `file '${escapeForConcat(p)}'`).join('\n')); + await writeFileGuarded(listFile, videoPaths.map((p) => `file '${escapeForConcat(p)}'`).join('\n')); }; const outFilename = `${filenamePrefix}-${id}.mp4`; @@ -186,7 +185,7 @@ export async function stitchVideos(videoIds, opts = {}) { } await optimizeForStreaming(outPath); } finally { - if (listFileWritten) await unlink(listFile).catch(() => {}); + if (listFileWritten) await unlinkGuarded(listFile).catch(() => {}); } const thumb = await generateThumbnail(outPath, id); diff --git a/server/services/videoGen/upscaleVideo.js b/server/services/videoGen/upscaleVideo.js index d1033f9fd2..372465a85f 100644 --- a/server/services/videoGen/upscaleVideo.js +++ b/server/services/videoGen/upscaleVideo.js @@ -1,10 +1,9 @@ /** Non-destructive 2x upscaling for video-history items. */ import { existsSync } from 'fs'; -import { unlink, copyFile } from 'fs/promises'; import { join } from 'path'; import { randomUUID } from 'crypto'; -import { PATHS, UUID_RE } from '../../lib/fileUtils.js'; +import { PATHS, UUID_RE, copyFileGuarded, unlinkGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { safeUnder, generateThumbnail, upscaleVideo2x } from '../../lib/ffmpeg.js'; import { loadHistory, mutateVideoHistory } from './history.js'; @@ -50,11 +49,11 @@ export async function upscaleHistoryItem(historyId) { // Copy first, then upscale-in-place — keeps the upscaler's atomic-rename // contract intact and means a mid-process kill leaves the source clip // untouched. - await copyFile(sourcePath, newPath); + await copyFileGuarded(sourcePath, newPath); console.log(`🔍 Upscaling video [${historyId.slice(0, 8)} → ${newId.slice(0, 8)}]: 2×`); const result = await upscaleVideo2x(newPath); if (!result.ok) { - await unlink(newPath).catch(() => {}); + await unlinkGuarded(newPath).catch(() => {}); throw new ServerError(`Upscale failed: ${result.reason}`, { status: 500, code: 'FFMPEG_FAILED' }); } const thumbnail = await generateThumbnail(newPath, newId); diff --git a/server/services/videoUpload.js b/server/services/videoUpload.js index d2509c68b6..7b2daf24f2 100644 --- a/server/services/videoUpload.js +++ b/server/services/videoUpload.js @@ -21,9 +21,8 @@ import { join } from 'path'; import { randomUUID } from 'crypto'; -import { writeFile, unlink } from 'fs/promises'; import { ServerError } from '../lib/errorHandler.js'; -import { PATHS, ensureDir } from '../lib/fileUtils.js'; +import { PATHS, ensureDir, atomicWrite, unlinkGuarded } from '../lib/fileUtils.js'; import { MAX_BASE64_UPLOAD_BYTES } from '../lib/uploadLimits.js'; import { generateThumbnail, probeVideoDuration } from '../lib/ffmpeg.js'; import { mutateVideoHistory } from './videoGen/history.js'; @@ -85,7 +84,7 @@ export async function saveUploadedGalleryVideoBuffer(buffer, originalName = '') const filename = `${id}.${ext}`; await ensureDir(PATHS.videos); const outPath = join(PATHS.videos, filename); - await writeFile(outPath, buffer); + await atomicWrite(outPath, buffer); try { // Both best-effort: a missing ffmpeg/ffprobe degrades to a thumbnail-less // entry (normalizeVideo renders a no-preview tile), never a failed upload. @@ -107,7 +106,7 @@ export async function saveUploadedGalleryVideoBuffer(buffer, originalName = '') // A throw between the byte write and the history write would orphan a // large file in data/videos with nothing pointing at it — mirror // downloadVideoIntoLibrary's cleanup-then-rethrow. - await unlink(outPath).catch(() => {}); + await unlinkGuarded(outPath).catch(() => {}); throw err; } } diff --git a/server/services/voice/profiles.js b/server/services/voice/profiles.js index 31f4619c7b..2768878f1c 100644 --- a/server/services/voice/profiles.js +++ b/server/services/voice/profiles.js @@ -8,11 +8,12 @@ */ import { createHash, randomUUID } from 'node:crypto'; -import { mkdir, writeFile } from 'node:fs/promises'; +import { mkdir } from 'node:fs/promises'; import { join } from 'node:path'; import { query } from '../../lib/db.js'; import { ServerError } from '../../lib/errorHandler.js'; import { PATHS } from '../../lib/paths.js'; +import { writeFileGuarded } from '../../lib/fileUtils.js'; export const VOICE_PROFILE_ENGINES = new Set(['kokoro', 'piper', 'qwen3-tts']); export const VOICE_PROFILE_KINDS = new Set(['preset', 'designed', 'cloned', 'fine-tuned']); @@ -353,7 +354,7 @@ export async function createClonedVoiceCandidate({ await mkdir(sourceDir, { recursive: true }); const safePath = join(sourceDir, cleanFilename); - await writeFile(safePath, audioBuffer); + await writeFileGuarded(safePath, audioBuffer); const sha256 = createHash('sha256').update(audioBuffer).digest('hex'); const now = timestamp(); diff --git a/server/services/writersRoom/evaluator.js b/server/services/writersRoom/evaluator.js index db35e9d15b..c60c140117 100644 --- a/server/services/writersRoom/evaluator.js +++ b/server/services/writersRoom/evaluator.js @@ -5,8 +5,8 @@ */ import { join } from 'path'; -import { readFile, readdir, rm } from 'fs/promises'; -import { PATHS, atomicWrite, ensureDir, safeJSONParse, tryReadFile } from '../../lib/fileUtils.js'; +import { readFile, readdir } from 'fs/promises'; +import { PATHS, atomicWrite, ensureDir, safeJSONParse, tryReadFile, rmGuarded } from '../../lib/fileUtils.js'; import { ServerError } from '../../lib/errorHandler.js'; import { stripCodeFences } from '../aiProvider.js'; import { runStagedLLM } from '../stageRunner.js'; @@ -267,7 +267,7 @@ async function migrateLegacyAnalyses(workId) { for (const [kind, { snapshot }] of latestPerKind) { await saveAnalysis(workId, { ...snapshot, id: kind }); } - await Promise.all(legacy.map((id) => rm(join(dir, `${id}.json`)).catch(() => {}))); + await Promise.all(legacy.map((id) => rmGuarded(join(dir, `${id}.json`)).catch(() => {}))); return legacy.length; } diff --git a/server/services/writersRoom/local.js b/server/services/writersRoom/local.js index 797110a58c..671ad96e75 100644 --- a/server/services/writersRoom/local.js +++ b/server/services/writersRoom/local.js @@ -15,8 +15,8 @@ */ import { randomUUID, createHash } from 'crypto'; -import { readFile, rm } from 'fs/promises'; -import { atomicWrite, ensureDir } from '../../lib/fileUtils.js'; +import { readFile } from 'fs/promises'; +import { atomicWrite, ensureDir, rmGuarded } from '../../lib/fileUtils.js'; import { countWords } from '../../lib/textUtils.js'; import { WORK_KINDS, WORK_STATUSES } from '../../lib/writersRoomPresets.js'; import { sanitizeVoiceExemplars, renderVoiceExemplars } from '../../lib/styleGuide.js'; @@ -532,7 +532,7 @@ export async function deleteWork(id) { // would drop it), so it was never syncable. Hard-remove the dir so the API's // "delete a broken work to recover" path still works (the soft-delete store // call would otherwise no-op on the unreadable manifest and strand it). - await rm(wrWorkDir(id), { recursive: true, force: true }); + await rmGuarded(wrWorkDir(id), { recursive: true, force: true }); return { ok: true }; } // Soft-delete tombstone (#1565) so the deletion federates and an out-of-date diff --git a/server/services/writersRoom/sync.js b/server/services/writersRoom/sync.js index 582986d116..63e33436ca 100644 --- a/server/services/writersRoom/sync.js +++ b/server/services/writersRoom/sync.js @@ -14,9 +14,8 @@ * from here, so importing it back would close a load-order cycle. */ -import { rm } from 'fs/promises'; import { existsSync } from 'fs'; -import { sha256File } from '../../lib/fileUtils.js'; +import { sha256File, rmGuarded } from '../../lib/fileUtils.js'; import { deleteSyncBaseHash, withBaseHashFlushBatch } from '../../lib/conflictJournal.js'; import { writersRoomStore } from './store.js'; import { @@ -68,7 +67,7 @@ export async function pruneTombstonedWorks(olderThanMs) { await withBaseHashFlushBatch(async () => { for (const id of ids) { if (typeof id !== 'string' || !WORK_ID_RE.test(id)) continue; - await rm(wrWorkDir(id), { recursive: true, force: true }).catch(() => {}); + await rmGuarded(wrWorkDir(id), { recursive: true, force: true }).catch(() => {}); await deleteSyncBaseHash(WRITERS_ROOM_WORK_KIND, id); } }); From 93d1810b8d6c51f2bd24d288bedd93ffec08a175 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Thu, 3 Sep 2026 23:41:33 -0700 Subject: [PATCH 002/194] feat([issue-6208]): emit only changed reviewer fields so task overrides inherit defaults ReviewerPicker gains a defaults prop and omits any key deep-equal to it, so touching one control no longer freezes the defaults-of-that-moment into permanent task metadata. GlobalConfigControls passes the same reviewDefaults/seededPins it seeds display from and rebuilds the reviewer slice from the partial payload (reverted keys are deleted, not rewritten). CodeReviewersTab passes no defaults and keeps the full emit. --- client/src/components/cos/ReviewerPicker.jsx | 99 ++++++++++++-- .../components/cos/ReviewerPicker.test.jsx | 125 ++++++++++++++++++ .../tabs/schedule/GlobalConfigControls.jsx | 33 ++++- .../GlobalConfigControlsReviewers.test.jsx | 109 +++++++++++++++ 4 files changed, 350 insertions(+), 16 deletions(-) create mode 100644 client/src/components/cos/tabs/schedule/GlobalConfigControlsReviewers.test.jsx diff --git a/client/src/components/cos/ReviewerPicker.jsx b/client/src/components/cos/ReviewerPicker.jsx index 118f10a6d1..3c05b6d964 100644 --- a/client/src/components/cos/ReviewerPicker.jsx +++ b/client/src/components/cos/ReviewerPicker.jsx @@ -58,11 +58,34 @@ const CUSTOM_MODEL_OPTION = '[custom]'; * - **Max Iterations** → the numeric `~max=` round cap (blank = slashdo's * built-in default, `0` = loop until clean). * - * Controlled: emits the full next shape via onChange so the parent can store + * Controlled: emits the next shape via onChange so the parent can store * `reviewers` / `usernames` / `optionalReviewers` / `reviewerModels` / * `reviewerEfforts` / `reviewerMaxRounds` / `reviewStopMode` / `reviewerApplies` * however it persists them. * + * `defaults` is the resolved fallback the parent seeded the props from (the + * install-wide Code Review Defaults, as token-keyed maps for the pins). When + * provided, `emit()` OMITS any key whose value is deep-equal to + * `defaults[key]` — a field the user never touched stays absent from the + * payload, which is exactly what `resolveReviewerConfig` reads as "inherit". + * Without it the picker would freeze the defaults-of-that-moment into a + * permanent task override on first touch (#6208). Omit the prop where a full + * emit is correct — the surface that edits the defaults themselves has no + * fallback to inherit from. + * + * Absent keeps meaning inherit and an explicitly-empty value keeps meaning + * "clear": the comparison is against the resolved default, never against + * emptiness, so `{}` / `[]` equal only a matching default and are otherwise + * emitted as a real override that clears it. (One key is exempt: the server + * drops an empty `reviewers` list before persisting, so `reviewers: []` + * resolves to the default chain either way — pre-existing server behavior.) + * + * Known trade-off: the diff is against the CURRENT default, so an override + * that happens to equal it (e.g. set before the default changed to match) is + * indistinguishable from "never touched" and reverts to inherit the next time + * any other field is edited. The effective reviewers are unchanged at that + * moment — the pin only stops shadowing future default changes. + * * `modelOptions` is the resolved model-picker data, shaped like * `useReviewerModelOptions()`'s return: `{ optionsByReviewer, defaultModels, * freeText, unavailable, providerDisabled, loaded }`. Callers keep owning their @@ -101,6 +124,7 @@ export default function ReviewerPicker({ installed = null, stopMode = DEFAULT_REVIEW_STOP_MODE, reviewerApplies = false, + defaults = null, onChange, disabled = false, showRunFlags = true @@ -221,17 +245,68 @@ export default function ReviewerPicker({ ? addable : addable.filter(opt => !hiddenAddable.includes(opt)); - const emit = (next) => onChange?.({ - reviewers: selected, - usernames: selectedUsernames, - optionalReviewers: optionalTokens, - reviewerMaxRounds: maxRoundsMap, - reviewerModels: modelsMap, - reviewerEfforts: effortsMap, - stopMode, - reviewerApplies, - ...next - }); + // Case-insensitive equality for the token lists (reviewer slugs are already + // lowercased; GitHub usernames are case-insensitive). Order matters ONLY for + // `reviewers` — the chain runs in click order — so the username lists compare + // as sorted sets; otherwise a same-membership reorder would over-emit. + const listsEqual = (a, b, ordered) => { + if (!Array.isArray(a) || !Array.isArray(b)) return a === b; + if (a.length !== b.length) return false; + const left = a.map((value) => String(value).toLowerCase()); + const right = b.map((value) => String(value).toLowerCase()); + if (!ordered) { + left.sort(); + right.sort(); + } + return left.every((value, index) => value === right[index]); + }; + // Case-insensitive-key equality for the token-keyed pin maps. Values compare + // strictly: `0` (loop until clean) must never equal absent, and `{}` equals + // only a matching default so an explicit clear is still emitted. + const mapsEqual = (a, b) => { + const entries = (map) => { + if (!map || typeof map !== 'object' || Array.isArray(map)) return map; + return Object.entries(map) + .map(([key, value]) => [key.toLowerCase(), value]) + .sort(([x], [y]) => (x < y ? -1 : x > y ? 1 : 0)); + }; + const left = entries(a); + const right = entries(b); + if (!Array.isArray(left) || !Array.isArray(right)) return left === right; + if (left.length !== right.length) return false; + return left.every(([key, value], index) => right[index][0] === key && right[index][1] === value); + }; + const equalsBaseline = (key, value, baseline) => { + if (key === 'reviewers') return listsEqual(value, baseline, true); + if (key === 'usernames' || key === 'optionalReviewers') return listsEqual(value, baseline, false); + if (key === 'reviewerMaxRounds' || key === 'reviewerModels' || key === 'reviewerEfforts') return mapsEqual(value, baseline); + return value === baseline; + }; + + const emit = (next) => { + const full = { + reviewers: selected, + usernames: selectedUsernames, + optionalReviewers: optionalTokens, + reviewerMaxRounds: maxRoundsMap, + reviewerModels: modelsMap, + reviewerEfforts: effortsMap, + stopMode, + reviewerApplies, + ...next + }; + // No baseline (the surface editing the defaults themselves): full snapshot, + // exactly as before. + if (!defaults || typeof defaults !== 'object') { + onChange?.(full); + return; + } + const partial = {}; + for (const key of Object.keys(full)) { + if (!equalsBaseline(key, full[key], defaults[key])) partial[key] = full[key]; + } + onChange?.(partial); + }; const toggleOptional = (token) => emit({ optionalReviewers: isOptional(token) ? withoutToken(token) : [...optionalTokens, token] diff --git a/client/src/components/cos/ReviewerPicker.test.jsx b/client/src/components/cos/ReviewerPicker.test.jsx index 427144c5be..e24a18f5dc 100644 --- a/client/src/components/cos/ReviewerPicker.test.jsx +++ b/client/src/components/cos/ReviewerPicker.test.jsx @@ -668,4 +668,129 @@ describe('ReviewerPicker', () => { expect(details).toHaveAttribute('open'); }); }); + + describe('defaults-aware emit (#6208)', () => { + const DEFAULTS = { + reviewers: ['copilot'], + usernames: [], + optionalReviewers: [], + reviewerMaxRounds: {}, + reviewerModels: {}, + reviewerEfforts: {}, + stopMode: 'all', + reviewerApplies: false, + }; + + it('emits the full snapshot when no defaults are provided', async () => { + const onChange = vi.fn(); + const user = userEvent.setup(); + render(); + await user.click(screen.getByLabelText('Remove Codex')); + expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ + reviewers: ['antigravity'], + usernames: [], + optionalReviewers: [], + reviewerMaxRounds: {}, + reviewerModels: {}, + reviewerEfforts: {}, + })); + }); + + it('omits every key that still equals the defaults when only the stop-mode changes', async () => { + const onChange = vi.fn(); + const user = userEvent.setup(); + render( + + ); + await user.selectOptions(screen.getByLabelText('Stop mode:'), 'on-clean'); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith({ stopMode: 'on-clean' }); + }); + + it('emits only reviewers when one is removed from a seeded list', async () => { + const onChange = vi.fn(); + const user = userEvent.setup(); + render( + + ); + await user.click(screen.getByLabelText('Remove Codex')); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith({ reviewers: ['antigravity'] }); + }); + + it('emits an explicitly-emptied map that clears a default pin (absent ≠ empty)', async () => { + const onChange = vi.fn(); + const user = userEvent.setup(); + render( + + ); + await user.click(screen.getByLabelText('Make Codex blocking')); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith({ optionalReviewers: [] }); + }); + + it('omits a pin map that already matches the defaults when an unrelated control changes', async () => { + const onChange = vi.fn(); + const user = userEvent.setup(); + render( + + ); + await user.click(screen.getByLabelText('Make Codex non-blocking')); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith({ optionalReviewers: ['codex'] }); + }); + + it('compares membership lists order-insensitively (reviewers stay order-sensitive)', async () => { + const onChange = vi.fn(); + const user = userEvent.setup(); + render( + + ); + // Reordering the reviewers IS a change (run order); the same-membership + // optional set in another order is not — only reviewers is emitted. + await user.click(screen.getByLabelText('Move Antigravity earlier')); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith({ reviewers: ['antigravity', 'codex'] }); + }); + + it('compares pin-map keys case-insensitively', async () => { + const onChange = vi.fn(); + const user = userEvent.setup(); + render( + + ); + await user.click(screen.getByLabelText('Make Codex non-blocking')); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith({ optionalReviewers: ['codex'] }); + }); + }); }); diff --git a/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx b/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx index a81269b5d7..40ef816729 100644 --- a/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx +++ b/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx @@ -675,13 +675,38 @@ export default function GlobalConfigControls({ taskType, config, onUpdate, onTri reviewerApplies={config.taskMetadata?.reviewerApplies !== undefined ? (config.taskMetadata?.reviewerApplies === true || config.taskMetadata?.reviewerApplies === 'true') : reviewDefaults.reviewerApplies} + // The same fallback the props above were seeded from. The picker + // omits whatever still equals it, so touching one control no + // longer freezes the rest into a permanent task override (#6208). + defaults={{ + reviewers: reviewDefaults.reviewers, + usernames: reviewDefaults.usernames, + optionalReviewers: reviewDefaults.optionalReviewers, + reviewerMaxRounds: reviewDefaults.reviewerMaxRounds, + reviewerModels: seededPins.models, + reviewerEfforts: seededPins.efforts, + stopMode: reviewDefaults.stopMode, + reviewerApplies: reviewDefaults.reviewerApplies, + }} disabled={updating} - onChange={({ reviewers, usernames, optionalReviewers, reviewerMaxRounds, reviewerModels, reviewerEfforts, stopMode, reviewerApplies }) => { + onChange={(patch) => { + // The picker emits only what differs from `defaults` above, so + // rebuild the reviewer slice from scratch: strip every override + // key (a key that reverted to the default must be DELETED, not + // left pinning its old value), then re-apply what arrived. // Drop the legacy single `reviewer` key so storage converges on `reviewers`. const { reviewer: _reviewer, ...rest } = config.taskMetadata || {}; - onUpdate(taskType, { - taskMetadata: { ...rest, reviewers, usernames, optionalReviewers, reviewerMaxRounds, reviewerModels, reviewerEfforts, reviewStopMode: stopMode, reviewerApplies } - }); + for (const key of REVIEW_CONFIG_KEYS) delete rest[key]; + const taskMetadata = { ...rest }; + if (patch.reviewers !== undefined) taskMetadata.reviewers = patch.reviewers; + if (patch.usernames !== undefined) taskMetadata.usernames = patch.usernames; + if (patch.optionalReviewers !== undefined) taskMetadata.optionalReviewers = patch.optionalReviewers; + if (patch.reviewerMaxRounds !== undefined) taskMetadata.reviewerMaxRounds = patch.reviewerMaxRounds; + if (patch.reviewerModels !== undefined) taskMetadata.reviewerModels = patch.reviewerModels; + if (patch.reviewerEfforts !== undefined) taskMetadata.reviewerEfforts = patch.reviewerEfforts; + if (patch.stopMode !== undefined) taskMetadata.reviewStopMode = patch.stopMode; + if (patch.reviewerApplies !== undefined) taskMetadata.reviewerApplies = patch.reviewerApplies; + onUpdate(taskType, { taskMetadata }); }} /> diff --git a/client/src/components/cos/tabs/schedule/GlobalConfigControlsReviewers.test.jsx b/client/src/components/cos/tabs/schedule/GlobalConfigControlsReviewers.test.jsx new file mode 100644 index 0000000000..ae837fcd9c --- /dev/null +++ b/client/src/components/cos/tabs/schedule/GlobalConfigControlsReviewers.test.jsx @@ -0,0 +1,109 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +// Same hook stubs as GlobalConfigControls.test.jsx, but ReviewerPicker is REAL +// here — these tests prove the picker + controls emit only what changed (#6208). +vi.mock('../../../../hooks/useCodeReviewDefaults', () => ({ + useCodeReviewDefaults: () => ({ + reviewers: ['codex', 'antigravity'], + usernames: [], + optionalReviewers: [], + reviewerMaxRounds: {}, + stopMode: 'all', + reviewerApplies: false, + }), +})); +vi.mock('../../../../hooks/useReviewerModelOptions', () => ({ + default: () => ({ optionsByReviewer: {}, freeText: {}, unavailable: {}, loaded: true }), +})); + +import GlobalConfigControls from './GlobalConfigControls'; + +const BASE_CONFIG = { + type: 'cron', + cronExpression: '0 7 * * *', + enabled: true, + providerId: null, + model: null, + effort: null, + prompt: 'do the thing', + status: {}, +}; + +function renderControls({ taskMetadata, onUpdate = vi.fn(async () => {}), taskType = 'claim-work' } = {}) { + render( + {}} + providers={[]} + apps={[]} + updating={false} + setUpdating={() => {}} + allTaskTypes={['claim-work']} + /> + ); + return onUpdate; +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('GlobalConfigControls — reviewer override emits only what changed (#6208)', () => { + it('changing only the stop-mode on a task with no reviewer override persists reviewStopMode and nothing else', async () => { + const onUpdate = renderControls({ + taskMetadata: { useWorktree: false, openPR: false, claimFlow: true }, + }); + const user = userEvent.setup(); + await user.selectOptions(screen.getByLabelText('Stop mode:'), 'on-clean'); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('claim-work', { + taskMetadata: { useWorktree: false, openPR: false, claimFlow: true, reviewStopMode: 'on-clean' }, + }); + }); + + it('removing one reviewer from a seeded list persists reviewers and nothing else', async () => { + const onUpdate = renderControls({ + taskMetadata: { useWorktree: false, openPR: false, claimFlow: true }, + }); + const user = userEvent.setup(); + await user.click(screen.getByLabelText('Remove Codex')); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('claim-work', { + taskMetadata: { useWorktree: false, openPR: false, claimFlow: true, reviewers: ['antigravity'] }, + }); + }); + + it('reverting a pin to the default deletes the key instead of persisting the snapshot', async () => { + // Task carries a stale full-snapshot override; removing the extra reviewer + // back to the seeded list must drop the reviewers key, not rewrite it. + const onUpdate = renderControls({ + taskMetadata: { + useWorktree: false, + openPR: false, + claimFlow: true, + reviewers: ['codex', 'antigravity', 'copilot'], + }, + }); + const user = userEvent.setup(); + await user.click(screen.getByLabelText('Remove Copilot')); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('claim-work', { + taskMetadata: { useWorktree: false, openPR: false, claimFlow: true }, + }); + }); + + it('the reset button still clears a stop-mode-only override', () => { + const onUpdate = renderControls({ + taskMetadata: { useWorktree: false, openPR: false, claimFlow: true, reviewStopMode: 'on-clean' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Use system Code Review Defaults' })); + expect(onUpdate).toHaveBeenCalledWith('claim-work', { + taskMetadata: { useWorktree: false, openPR: false, claimFlow: true }, + }); + }); +}); From acbccd10c0ea609c1fefdfa224ba6f92c82d94ce Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 4 Sep 2026 07:01:54 +0000 Subject: [PATCH 003/194] fix(cos): warn when pending on-demand tasks can't run because the daemon is stopped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit triggerOnDemandTask() writes a request to schedule.onDemandRequests regardless of daemon state, but dequeueNextTask() bails immediately when isDaemonRunning() is false — so a stopped daemon leaves the "Pending On-Demand Tasks" banner showing a request that will never be picked up, with no indication why. Pass the already-live status.running down from ChiefOfStaff so the banner switches to a warning and the queue toast tells the user the daemon needs to be started, instead of looking silently stuck. --- .../src/components/cos/tabs/ScheduleTab.jsx | 26 ++++++--- .../components/cos/tabs/ScheduleTab.test.jsx | 54 +++++++++++++++++++ client/src/pages/ChiefOfStaff.jsx | 2 +- 3 files changed, 75 insertions(+), 7 deletions(-) diff --git a/client/src/components/cos/tabs/ScheduleTab.jsx b/client/src/components/cos/tabs/ScheduleTab.jsx index 04a56ad54b..bc2673fdcf 100644 --- a/client/src/components/cos/tabs/ScheduleTab.jsx +++ b/client/src/components/cos/tabs/ScheduleTab.jsx @@ -3,7 +3,7 @@ import { useSearchParams } from 'react-router'; import { AlertCircle, RefreshCw } from 'lucide-react'; import toast from '../../ui/Toast'; import * as api from '../../../services/api'; -import { formatDateTime, formatTimeOfDaySeconds } from '../../../utils/formatters'; +import { formatDateTime, formatTimeOfDaySeconds, timeAgo } from '../../../utils/formatters'; import Banner from '../../ui/Banner'; import { CodeReviewDefaultsProvider } from '../../../hooks/useCodeReviewDefaults'; import { useAppOverrideActions } from '../../../hooks/useAppOverrideActions'; @@ -40,7 +40,7 @@ function mergeOnDemandRequest(schedule, request) { // passed down — same convention as TasksTab/AgentsTab — so this tab's provider/ // model pickers stay live without standing up a second independent poll of the // same data. -export default function ScheduleTab({ apps, providers, providersLoaded, activeProviderId }) { +export default function ScheduleTab({ apps, providers, providersLoaded, activeProviderId, daemonRunning }) { const [searchParams, setSearchParams] = useSearchParams(); const [schedule, setSchedule] = useState(null); const [loading, setLoading] = useState(true); @@ -101,7 +101,12 @@ export default function ScheduleTab({ apps, providers, providersLoaded, activePr if (!result?.success) return null; const appName = appId ? apps?.find(app => app.id === appId)?.name || 'selected app' : null; - toast.success(`Queued ${taskType} request${appName ? ` for ${appName}` : ''} — it will appear in Tasks when evaluation begins`); + const queuedMsg = `Queued ${taskType} request${appName ? ` for ${appName}` : ''}`; + if (daemonRunning === false) { + toast.error(`${queuedMsg} — but the CoS daemon is stopped, so it will not run until you start it`); + } else { + toast.success(`${queuedMsg} — it will appear in Tasks when evaluation begins`); + } // The POST returns the persisted request. Paint it immediately instead of // waiting for a second round trip; the evaluator may drain it into Tasks @@ -112,7 +117,7 @@ export default function ScheduleTab({ apps, providers, providersLoaded, activePr } fetchSchedule(); return result.request || true; - }, [apps, fetchSchedule]); + }, [apps, fetchSchedule, daemonRunning]); const handleTriggerAppImprovement = handleTriggerTask; @@ -170,11 +175,20 @@ export default function ScheduleTab({ apps, providers, providersLoaded, activePr )} {schedule.onDemandRequests?.length > 0 && ( - + + {daemonRunning === false && ( +
+ The CoS daemon is stopped, so these requests will not run until it's started — use the Start button above. +
+ )}
{schedule.onDemandRequests.map(req => (
- {req.taskType}{req.appId ? ` (${apps?.find(app => app.id === req.appId)?.name || req.appId})` : ''} - requested {formatTimeOfDaySeconds(req.requestedAt)} + {req.taskType}{req.appId ? ` (${apps?.find(app => app.id === req.appId)?.name || req.appId})` : ''} - requested {formatTimeOfDaySeconds(req.requestedAt)} ({timeAgo(req.requestedAt)})
))}
diff --git a/client/src/components/cos/tabs/ScheduleTab.test.jsx b/client/src/components/cos/tabs/ScheduleTab.test.jsx index b5a40fbdf2..987935ea60 100644 --- a/client/src/components/cos/tabs/ScheduleTab.test.jsx +++ b/client/src/components/cos/tabs/ScheduleTab.test.jsx @@ -98,4 +98,58 @@ describe('ScheduleTab on-demand feedback', () => { expect(screen.getByText('Pending On-Demand Tasks')).toBeVisible(); expect(screen.getByText(/review \(Example App\) - requested/)).toBeVisible(); }); + + it('warns instead of celebrating a queued request when the CoS daemon is stopped', async () => { + const user = userEvent.setup(); + const request = { + id: 'request-2', + taskType: 'review', + appId: 'app-1', + requestedAt: '2026-09-01T12:00:00.000Z', + }; + api.getCodeReviewDefaults.mockResolvedValue({}); + api.getCosSchedule + .mockResolvedValueOnce({ + improvementEnabled: true, + tasks: { + review: { + type: 'on-demand', + enabled: true, + enabledAppCount: 1, + totalAppCount: 1, + invocation: { userInvokable: true }, + }, + }, + onDemandRequests: [], + }) + // Hold the background refresh so this assertion proves the + // optimistically-painted request (and its daemon-stopped warning) + // stays visible without waiting on a second round trip. + .mockReturnValueOnce(new Promise(() => {})); + api.triggerCosOnDemandTask.mockResolvedValue({ success: true, request }); + + render( + + + , + ); + + await user.click(await screen.findByRole('button', { name: /Run on App/i })); + await user.click(screen.getByRole('button', { name: 'Example App' })); + + await waitFor(() => expect(api.triggerCosOnDemandTask).toHaveBeenCalledWith( + 'review', + 'app-1', + { silent: true }, + )); + expect(toast.error).toHaveBeenCalledWith( + 'Queued review request for Example App — but the CoS daemon is stopped, so it will not run until you start it', + ); + expect(await screen.findByText(/CoS daemon is stopped/)).toBeVisible(); + }); }); diff --git a/client/src/pages/ChiefOfStaff.jsx b/client/src/pages/ChiefOfStaff.jsx index 32f69a4426..e9b4fce27a 100644 --- a/client/src/pages/ChiefOfStaff.jsx +++ b/client/src/pages/ChiefOfStaff.jsx @@ -1237,7 +1237,7 @@ export default function ChiefOfStaff() { {activeTab === 'schedule' && (
}> - +
)} From 82d855ded1ca2b103a6245da68ab655751fee9c4 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 4 Sep 2026 07:06:11 +0000 Subject: [PATCH 004/194] fix: migrate LoRA downloads onto the shared download slot (#6190) loras.js was the only streamResumableDownload caller not registered with createDownloadSlot, so the orphaned-partial GC sweep couldn't see a live LoRA download's .partial (harmless today only because the age gate outlives any fresh transfer) and a second install of the same destination could race a parallel copy of the same file. Claims a slot keyed on the resolved destination path before the first await, wires an external abort signal (the SSE stream's client-disconnect handler) to cancel through the slot so it gets the same typed error as a stall, and releases in a finally. Not exclusive and keepPartialOnCancel defaults false, matching the spec-decode single-file precedent. --- server/lib/downloadPreflight.js | 3 +- server/services/loras.js | 149 ++++++++++++++++----------- server/services/loras.test.js | 113 ++++++++++++++++++-- server/services/orphanedPartialGc.js | 7 +- 4 files changed, 198 insertions(+), 74 deletions(-) diff --git a/server/lib/downloadPreflight.js b/server/lib/downloadPreflight.js index 4c60db550a..7ffb395c13 100644 --- a/server/lib/downloadPreflight.js +++ b/server/lib/downloadPreflight.js @@ -542,8 +542,7 @@ const IDLE_STALL_TIMEOUT_MS = 20 * 60 * 1000; * migrated download path stayed unprotected until somebody remembered to add * that clause. Registering here removes that step for every path that claims a * slot; what such a path must still do is BE LOADED, which - * `orphanedPartialGc.test.js` pins. (A path that claims no slot — `loras.js` - * today — is still unprotected; issue #6190 migrates it.) + * `orphanedPartialGc.test.js` pins. * * Call `createDownloadSlot` at MODULE scope only: this Set is add-only, so a * per-request slot would grow it for the process's lifetime and slow diff --git a/server/services/loras.js b/server/services/loras.js index 1e72456f74..0ce404428c 100644 --- a/server/services/loras.js +++ b/server/services/loras.js @@ -23,6 +23,7 @@ import { ServerError } from '../lib/errorHandler.js'; import { assessDownloadPreflight, assertDownloadFits, + createDownloadSlot, etagPathFor, probeRemoteSize, siblingDownloadMeta, @@ -82,6 +83,14 @@ const loraMetadataCache = new Map(); // files, while two explicitly different versions remain independent. const civitaiInstalls = createSingleFlight(); +// Keyed on the resolved destination path — what `isAnyDownloadInFlight` (and the +// orphaned-partial GC sweep) asks about, and what a second install of the same +// file would otherwise race. Not `exclusive`: LoRA files are small relative to a +// checkpoint, so installing several at once is normal, not a hazard. Cancel +// discards the `.partial` (`keepPartialOnCancel` defaults false) — a single +// abandoned file, not a multi-shard checkpoint worth resuming. +const downloadSlot = createDownloadSlot({ codePrefix: 'LORA' }); + const sidecarPath = (loraFilename) => join(PATHS.loras, `${loraFilename}${SIDECAR_SUFFIX}`); const invalidateLoraMetadataCache = (filename) => { loraMetadataCache.delete(join(PATHS.loras, filename)); @@ -465,70 +474,88 @@ const downloadToFile = async (url, destPath, { fetchImpl = fetch, headers = {} , throw new ServerError(`${label} download failed: ${res.status} ${res.statusText}`, { status: 502, code }); }; - let lastEmit = 0; - let lastTick = { received: 0, total: 0 }; - const { tmpPath } = await streamResumableDownload({ - url, - destPath, - headers, - fetchImpl, - signal, - finalize: false, - isCancelled: () => Boolean(signal?.aborted), - onHttpError, - onBytes: onProgress - ? (received, total) => { - lastTick = { received, total }; - const now = Date.now(); - if (now - lastEmit < 150) return; - lastEmit = now; - onProgress({ received, total }); - } - : undefined, + // Claim before the first await: two presses landing inside the same tick + // would otherwise both pass the caller's existsSync pre-check and start a + // parallel transfer of the same file. Released in the finally below. + const slot = downloadSlot.claim(destPath, { + busyMessage: `${basename(destPath)} is already downloading`, }); - // `finalize: false` means streamResumableDownload never had a chance to - // clean up its own etag sidecar (that only happens on ITS finalize path). - // Reaching here means the stream completed successfully — every branch - // below either moves or deletes tmpPath outright, never leaves it for a - // future resume, so the sidecar describing it is equally done. - await rmGuarded(etagPathFor(destPath), { force: true }).catch(() => {}); - if (onProgress) onProgress(lastTick); - // Atomic no-clobber finalize: `link` is POSIX-atomic and fails with EEXIST - // when destPath already exists (concurrent install that snuck past our - // pre-check). On success we unlink the tmp; on EEXIST we clean up and - // throw CIVITAI_ALREADY_INSTALLED. For other link errors (cross-device - // EXDEV, read-only fs, etc.) fall back to rename, which is the only - // portable option on those platforms. - const linkErr = await link(tmpPath, destPath).catch((e) => e); - if (!linkErr) { - await unlinkGuarded(tmpPath).catch(() => {}); - return; + // An external abort signal (SSE client-disconnect) cancels through the slot + // rather than its own controller, so the typed CANCELLED/STALLED error from + // `wrapError` below applies uniformly regardless of who triggered the abort. + const onExternalAbort = () => downloadSlot.cancel(destPath); + if (signal) { + if (signal.aborted) onExternalAbort(); + else signal.addEventListener('abort', onExternalAbort, { once: true }); } - if (linkErr.code === 'EEXIST') { - await rmGuarded(tmpPath, { force: true }).catch(() => {}); - const basename_ = basename(destPath); - throw new ServerError( - `Already installed: ${basename_}. Delete it first or pick a different version.`, - { status: 409, code: 'CIVITAI_ALREADY_INSTALLED' }, - ); - } - // EXDEV or similar — fall back to rename. Re-check destPath right before - // the rename so a concurrent install that landed between our link attempt - // and now can't be silently clobbered (POSIX rename overwrites). Treat - // late-arriving dest as CIVITAI_ALREADY_INSTALLED, matching the EEXIST - // path above. - if (existsSync(destPath)) { - await rmGuarded(tmpPath, { force: true }).catch(() => {}); - const basename_ = basename(destPath); - throw new ServerError( - `Already installed: ${basename_}. Delete it first or pick a different version.`, - { status: 409, code: 'CIVITAI_ALREADY_INSTALLED' }, - ); + const emitProgress = onProgress + ? slot.throttle((received, total) => onProgress({ received, total })) + : null; + try { + let lastTick = { received: 0, total: 0 }; + const { tmpPath } = await streamResumableDownload({ + url, + destPath, + headers, + fetchImpl, + finalize: false, + onHttpError, + onBytes: (received, total) => { + lastTick = { received, total }; + slot.track(received, total); + if (emitProgress) emitProgress(received, total); + }, + ...slot.downloadOptions(), + }); + // `finalize: false` means streamResumableDownload never had a chance to + // clean up its own etag sidecar (that only happens on ITS finalize path). + // Reaching here means the stream completed successfully — every branch + // below either moves or deletes tmpPath outright, never leaves it for a + // future resume, so the sidecar describing it is equally done. + await rmGuarded(etagPathFor(destPath), { force: true }).catch(() => {}); + if (onProgress) onProgress(lastTick); + // Atomic no-clobber finalize: `link` is POSIX-atomic and fails with EEXIST + // when destPath already exists (concurrent install that snuck past our + // pre-check). On success we unlink the tmp; on EEXIST we clean up and + // throw CIVITAI_ALREADY_INSTALLED. For other link errors (cross-device + // EXDEV, read-only fs, etc.) fall back to rename, which is the only + // portable option on those platforms. + const linkErr = await link(tmpPath, destPath).catch((e) => e); + if (!linkErr) { + await unlinkGuarded(tmpPath).catch(() => {}); + return; + } + if (linkErr.code === 'EEXIST') { + await rmGuarded(tmpPath, { force: true }).catch(() => {}); + const basename_ = basename(destPath); + throw new ServerError( + `Already installed: ${basename_}. Delete it first or pick a different version.`, + { status: 409, code: 'CIVITAI_ALREADY_INSTALLED' }, + ); + } + // EXDEV or similar — fall back to rename. Re-check destPath right before + // the rename so a concurrent install that landed between our link attempt + // and now can't be silently clobbered (POSIX rename overwrites). Treat + // late-arriving dest as CIVITAI_ALREADY_INSTALLED, matching the EEXIST + // path above. + if (existsSync(destPath)) { + await rmGuarded(tmpPath, { force: true }).catch(() => {}); + const basename_ = basename(destPath); + throw new ServerError( + `Already installed: ${basename_}. Delete it first or pick a different version.`, + { status: 409, code: 'CIVITAI_ALREADY_INSTALLED' }, + ); + } + await rename(tmpPath, destPath).catch(async (err) => { + await rmGuarded(tmpPath, { force: true }).catch(() => {}); + throw err; + }); + } catch (err) { + throw slot.wrapError(err); + } finally { + if (signal) signal.removeEventListener('abort', onExternalAbort); + slot.release(); } - await rename(tmpPath, destPath).catch(async (err) => { - await rmGuarded(tmpPath, { force: true }).catch(() => {}); - throw err; - }); }; // After a LoRA finishes downloading, verify the on-disk `.safetensors` before diff --git a/server/services/loras.test.js b/server/services/loras.test.js index 8591f8c01f..6f5810def0 100644 --- a/server/services/loras.test.js +++ b/server/services/loras.test.js @@ -988,25 +988,74 @@ describe('installFromHuggingface', () => { expect(ticks[ticks.length - 1]).toEqual({ received: 10, total: 0 }); }); - it('forwards an AbortSignal to the download fetch so an SSE disconnect can cancel it', async () => { + it('lets an external AbortSignal cancel the in-flight weights download so an SSE disconnect stops it', async () => { + // The download claims a slot keyed on destPath and downloads through the + // SLOT's own AbortController (so a stall/cancel gets the same typed error + // regardless of who triggered it) — an external `signal` (the route's + // res.on('close') controller) is wired to cancel THROUGH the slot rather + // than being handed to fetch directly. Prove the wiring by aborting mid- + // transfer and asserting the actual weights fetch's signal aborts too. const controller = new AbortController(); let sawSignal; const fetchImpl = async (url, opts) => { if (url.startsWith('https://huggingface.co/api/models/')) return mockJsonResponse(HF_MODEL); if (url.includes('/resolve/main/pytorch_lora_weights.safetensors')) { sawSignal = opts?.signal; - const stream = new ReadableStream({ start(c) { c.enqueue(new Uint8Array(validSafetensors())); c.close(); } }); - return { ok: true, status: 200, body: stream }; + // Never closes on its own: this stream only ends when the abort + // listener below errors it, mirroring how real fetch aborts an + // in-flight response body when its signal fires. + const stream = new ReadableStream({ + start(c) { + c.enqueue(new Uint8Array(validSafetensors().slice(0, 8))); + sawSignal.addEventListener('abort', () => c.error(new Error('aborted')), { once: true }); + }, + }); + return { ok: true, status: 200, body: stream, headers: new Map([['content-length', String(validSafetensors().length)]]) }; } throw new Error(`unexpected fetch: ${url}`); }; - await lorasService.installFromHuggingface( + const install = lorasService.installFromHuggingface( { url: 'https://huggingface.co/fal/ltx2.3-audio-reactive-lora', token: 'hf_test' }, { fetchImpl, signal: controller.signal }, ); - // The controller's signal must reach the actual weights download (not just - // the metadata fetch) — that's the transfer a disconnect needs to cancel. - expect(sawSignal).toBe(controller.signal); + await vi.waitFor(() => expect(sawSignal).toBeDefined()); + expect(sawSignal.aborted).toBe(false); + controller.abort(); + expect(sawSignal.aborted).toBe(true); + await expect(install).rejects.toThrow(/cancel/i); + }); + + it('refuses a second install of the same destination while the first is still downloading', async () => { + let release; + const gate = new Promise((resolve) => { release = resolve; }); + const full = validSafetensors(); + const fetchImpl = async (url) => { + if (url.startsWith('https://huggingface.co/api/models/')) return mockJsonResponse(HF_MODEL); + if (url.includes('/resolve/main/pytorch_lora_weights.safetensors')) { + const stream = new ReadableStream({ + start(c) { + c.enqueue(new Uint8Array(full.slice(0, 8))); + gate.then(() => { c.enqueue(new Uint8Array(full.slice(8))); c.close(); }); + }, + }); + return { ok: true, status: 200, body: stream, headers: new Map([['content-length', String(full.length)]]) }; + } + throw new Error(`unexpected fetch: ${url}`); + }; + const first = lorasService.installFromHuggingface( + { url: 'https://huggingface.co/fal/ltx2.3-audio-reactive-lora', token: 'hf_test' }, + { fetchImpl }, + ); + const destPath = join(tmpLoras, 'lora-fal-ltx2.3-audio-reactive-lora-hf.safetensors'); + await vi.waitFor(() => expect(existsSync(`${destPath}.partial`)).toBe(true)); + const err = await lorasService.installFromHuggingface( + { url: 'https://huggingface.co/fal/ltx2.3-audio-reactive-lora', token: 'hf_test' }, + { fetchImpl }, + ).catch((e) => e); + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('LORA_DOWNLOAD_IN_FLIGHT'); + release(); + await first; }); it('installs a Flux.2 Klein 9B collection as flux2 and picks the klein9b file', async () => { @@ -1113,6 +1162,56 @@ describe('installFromHuggingface', () => { }); }); +// #6190: loras.js is the third `streamResumableDownload` caller to register a +// slot — the orphaned-partial GC sweeps PATHS.loras but, before this, couldn't +// see a live LoRA download's `.partial` because loras.js claimed no slot. +describe('loras.js download-slot registration with the orphaned-partial GC', () => { + const HF_MODEL = { + id: 'fal/ltx2.3-audio-reactive-lora', + tags: ['ltxv', 'lora'], + cardData: { base_model: 'Lightricks/LTX-2.3', instance_prompt: 'audio reactive' }, + siblings: [ + { rfilename: 'README.md' }, + { rfilename: 'pytorch_lora_weights.safetensors' }, + ], + }; + const ANCIENT = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000); + + it('protects a live LoRA download from the sweep even once its .partial is aged past the gate', async () => { + const { sweepOrphanedDownloadPartials } = await import('./orphanedPartialGc.js'); + let release; + const gate = new Promise((resolve) => { release = resolve; }); + const fetchImpl = async (url) => { + if (url.startsWith('https://huggingface.co/api/models/')) return mockJsonResponse(HF_MODEL); + if (url.includes('/resolve/main/pytorch_lora_weights.safetensors')) { + const full = validSafetensors(); + const stream = new ReadableStream({ + start(c) { + c.enqueue(new Uint8Array(full.slice(0, 8))); + gate.then(() => { c.enqueue(new Uint8Array(full.slice(8))); c.close(); }); + }, + }); + return { ok: true, status: 200, body: stream, headers: new Map([['content-length', String(full.length)]]) }; + } + throw new Error(`unexpected fetch: ${url}`); + }; + const install = lorasService.installFromHuggingface( + { url: 'https://huggingface.co/fal/ltx2.3-audio-reactive-lora', token: 'hf_test' }, + { fetchImpl }, + ); + const partialPath = join(tmpLoras, 'lora-fal-ltx2.3-audio-reactive-lora-hf.safetensors.partial'); + await vi.waitFor(() => expect(existsSync(partialPath)).toBe(true)); + const fs = await import('fs/promises'); + await fs.utimes(partialPath, ANCIENT, ANCIENT); + + expect(await sweepOrphanedDownloadPartials({ dirs: [tmpLoras] })).toMatchObject({ deleted: 0, keptProtected: 1 }); + expect(existsSync(partialPath)).toBe(true); + + release(); + await install; + }); +}); + describe('LoRA key layout', () => { const writeLora = async (name, header) => { const fs = await import('fs/promises'); diff --git a/server/services/orphanedPartialGc.js b/server/services/orphanedPartialGc.js index 9f3b48c4fe..002ac530a6 100644 --- a/server/services/orphanedPartialGc.js +++ b/server/services/orphanedPartialGc.js @@ -20,11 +20,12 @@ import { resolveSpecModelPath } from './specDecodeModels.js'; import { getModelsDir as getOllamaModelsDir } from './ollamaManager.js'; import { getModelsDir as getLmStudioModelsDir } from './lmStudioManager.js'; import { slotstreamCacheDir } from '../lib/slotstreamModels.js'; -// Side-effect import: constructing the module is what registers its download +// Side-effect imports: constructing a module is what registers its download // slot with `isAnyDownloadInFlight`, and an unregistered slot's live shards are // unprotected from the sweep below. `orphanedPartialGc.test.js` fails if a // module calling `createDownloadSlot` stops being reachable from here. import './slotstreamModelManager.js'; +import './loras.js'; import { createSweepScheduler } from './sweepScheduler.js'; export { ORPHANED_PARTIAL_MAX_AGE_MS }; @@ -65,9 +66,7 @@ export async function sweepOrphanedDownloadPartials({ } = {}) { const targets = dirs || await collectPartialSweepDirs(); // One predicate for every runtime: each download slot registers itself, so a - // path that claims a slot is protected without a clause added here. (A path - // that claims none — `loras.js`, which writes into `PATHS.loras` above — is - // not; migrating it is issue #6190.) + // path that claims a slot is protected without a clause added here. return sweepOrphanedPartials(targets, { now, maxAgeMs, isProtected: isAnyDownloadInFlight }); } From d8b631410d1a87d2eb84a3ce0f69f4d135440fda Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 4 Sep 2026 07:08:11 +0000 Subject: [PATCH 005/194] test: assert the typed cancel code on the external-abort LoRA download test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strengthens the abort-through-slot test from the prior commit per opencode review feedback — asserts LORA_DOWNLOAD_CANCELLED, not just a message match. --- server/services/loras.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/services/loras.test.js b/server/services/loras.test.js index 6f5810def0..537d1f9173 100644 --- a/server/services/loras.test.js +++ b/server/services/loras.test.js @@ -1022,7 +1022,9 @@ describe('installFromHuggingface', () => { expect(sawSignal.aborted).toBe(false); controller.abort(); expect(sawSignal.aborted).toBe(true); - await expect(install).rejects.toThrow(/cancel/i); + const err = await install.catch((e) => e); + expect(err.message).toMatch(/cancel/i); + expect(err.code).toBe('LORA_DOWNLOAD_CANCELLED'); }); it('refuses a second install of the same destination while the first is still downloading', async () => { From e7d8921a44aa061ea72672fb9f2121d6ad03a41f Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 4 Sep 2026 07:33:46 +0000 Subject: [PATCH 006/194] feat: add fal.ai queue REST API video backend to MediaGen and FableLoom (#6213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds server/services/videoGen/fal.js — a queue-submit/poll/download provider mirroring videoGen/grok.js's job-map/SSE contract — as a first-class mediaJobQueue cloud-lane video backend, replacing the brittle Playwright browser automation FableLoom used for fal.ai (services/fableLoom/falVideoAutomation.js, left in place as a fallback) with a direct API call. Wires 'fal' through the VIDEO_GEN_MODE alphabet, the video pin ladder, prepareVideoGenParams' backend short-circuit, submitJob's dispatch, and the VideoGen page's backend switch, plus a Settings > Image Gen field for the FAL_KEY / API key. --- .env.example | 9 + .../fableloom/LoomSettingsDrawer.jsx | 1 + .../src/components/settings/ImageGenTab.jsx | 33 ++- .../components/settings/ImageGenTab.test.jsx | 4 +- client/src/hooks/useVideoGenFieldState.js | 4 + client/src/hooks/useVideoGenForm.js | 27 +- client/src/lib/imageGenModes.js | 3 +- client/src/lib/videoGenSubmission.js | 17 +- client/src/pages/VideoGen.jsx | 86 ++++-- server/lib/generationModes.js | 5 +- server/lib/generationModes.test.js | 8 +- server/lib/validation.js | 5 + server/routes/videoGen.js | 20 +- server/services/fableLoom/production.js | 29 +- server/services/mediaJobQueue/index.js | 6 +- server/services/videoGen/fal.js | 273 ++++++++++++++++++ server/services/videoGen/fal.test.js | 188 ++++++++++++ server/services/videoGen/modes.js | 6 + server/services/videoGen/modes.test.js | 14 +- server/services/videoGen/prepareParams.js | 30 +- server/services/videoGen/submitJob.js | 32 +- 21 files changed, 747 insertions(+), 53 deletions(-) create mode 100644 server/services/videoGen/fal.js create mode 100644 server/services/videoGen/fal.test.js diff --git a/.env.example b/.env.example index 7883c2ded6..9f2ba7e57d 100644 --- a/.env.example +++ b/.env.example @@ -110,6 +110,11 @@ PGPASSWORD=portos # GROK_TIMEOUT_MS — keep it above the 20-minute cloud-lane idle watchdog. # GROK_VIDEO_TIMEOUT_MS=1800000 +# Maximum fal.ai queue video-generation runtime in milliseconds (videoGen/fal.js; +# default: 1200000 / 20 minutes) — a queue render can sit behind other tenants' +# jobs before it starts, so this bounds the whole submit→poll→download run. +# FAL_VIDEO_TIMEOUT_MS=1200000 + # Maximum Antigravity image-generation runtime in milliseconds (default: 1200000 / 20 minutes) # AGY_IMAGEGEN_TIMEOUT_MS=1200000 @@ -150,6 +155,10 @@ PGPASSWORD=portos # CivitAI API key for LoRA / model downloads from civitai.com # CIVITAI_API_KEY=your_civitai_api_key_here +# fal.ai API key for the fal.ai queue video backend (Video Gen page, FableLoom). +# Settings > Video Gen's stored key wins when both are set. +# FAL_KEY=your_fal_api_key_here + # Absolute path to the bash binary used to run PortOS's bundled *.sh scripts # (e.g. scripts/db.sh). Auto-detected if unset; on Windows it prefers Git Bash, # because a bare `bash` often resolves to WSL, which can't see drive paths. diff --git a/client/src/components/fableloom/LoomSettingsDrawer.jsx b/client/src/components/fableloom/LoomSettingsDrawer.jsx index a588e1909b..69cdf3d544 100644 --- a/client/src/components/fableloom/LoomSettingsDrawer.jsx +++ b/client/src/components/fableloom/LoomSettingsDrawer.jsx @@ -51,6 +51,7 @@ const VIDEO_RENDER_BACKENDS = [ { id: 'auto', label: 'Auto', icon: Layers }, { id: 'local', label: 'Local', icon: Cpu }, { id: 'grok', label: 'Grok', icon: Cloud }, + { id: 'fal', label: 'fal.ai', icon: Cloud }, ]; const modelOptions = (models, selected) => { diff --git a/client/src/components/settings/ImageGenTab.jsx b/client/src/components/settings/ImageGenTab.jsx index 132cd08e6d..ab091439fd 100644 --- a/client/src/components/settings/ImageGenTab.jsx +++ b/client/src/components/settings/ImageGenTab.jsx @@ -112,6 +112,10 @@ export function ImageGenTab() { // (defaultModelId) survive the settings PUT's wholesale slice replace. const [videoGenMode, setVideoGenMode] = useState(''); const [videoGenDisplaySleep, setVideoGenDisplaySleep] = useState(true); + // fal.ai queue REST API key (#6213) — usability-gated on this being set + // (settings, or the FAL_KEY env var server-side). No enabled toggle: the + // key's presence IS the opt-in, same shape as loras.js's Civitai key. + const [falApiKey, setFalApiKey] = useState(''); const videoGenSliceRef = useRef({}); const [sdapiUrl, setSdapiUrl] = useState(''); const [pythonPath, setPythonPath] = useState(''); @@ -177,6 +181,7 @@ export function ImageGenTab() { renderDefaultsJson: '{}', videoGenMode: '', videoGenDisplaySleep: true, + falApiKey: '', }); const [status, setStatus] = useState(null); @@ -249,6 +254,7 @@ export function ImageGenTab() { const vg = (s?.videoGen && typeof s.videoGen === 'object') ? s.videoGen : {}; const vgMode = normalizeRenderPinValue(vg.mode) || ''; const vgDisplaySleep = vg.displaySleep !== false; + const vgFalApiKey = vg.fal?.apiKey || ''; const m = ig.mode || IMAGE_GEN_MODE.EXTERNAL; const url = normalizeUrl(ig.external?.sdapiUrl || ig.sdapiUrl); const py = ig.local?.pythonPath || ''; @@ -284,6 +290,7 @@ export function ImageGenTab() { setRenderDefaults(rd); setVideoGenMode(vgMode); setVideoGenDisplaySleep(vgDisplaySleep); + setFalApiKey(vgFalApiKey); videoGenSliceRef.current = vg; setSdapiUrl(url); setPythonPath(py); @@ -312,6 +319,7 @@ export function ImageGenTab() { renderDefaultsJson: JSON.stringify(rd), videoGenMode: vgMode, videoGenDisplaySleep: vgDisplaySleep, + falApiKey: vgFalApiKey, }); setToolRegistered(tools.some((t) => t.id === SDAPI_TOOL_ID)); setCodexToolRegistered(tools.some((t) => t.id === CODEX_TOOL_ID)); @@ -403,7 +411,8 @@ export function ImageGenTab() { || denoiseByMode.external !== saved.denoiseByMode.external || JSON.stringify(renderDefaults) !== saved.renderDefaultsJson || videoGenMode !== saved.videoGenMode - || videoGenDisplaySleep !== saved.videoGenDisplaySleep; + || videoGenDisplaySleep !== saved.videoGenDisplaySleep + || falApiKey !== saved.falApiKey; const handleSave = async () => { setSaving(true); @@ -448,7 +457,12 @@ export function ImageGenTab() { ), // Install-wide video pin (#3231 Phase 4). Spread over the loaded slice so // sibling keys (defaultModelId) survive the wholesale slice replace. - videoGen: { ...videoGenSliceRef.current, mode: videoGenMode || null, displaySleep: videoGenDisplaySleep }, + videoGen: { + ...videoGenSliceRef.current, + mode: videoGenMode || null, + displaySleep: videoGenDisplaySleep, + fal: { ...videoGenSliceRef.current.fal, apiKey: falApiKey.trim() }, + }, }; try { await updateSettings(patch, { silent: true }); @@ -466,6 +480,7 @@ export function ImageGenTab() { renderDefaultsJson: JSON.stringify(patch.renderDefaults), videoGenMode, videoGenDisplaySleep, + falApiKey: falApiKey.trim(), }); // Reflect the pruned no-op entries back into the editor state so the // dirty check compares like against like after a save. @@ -770,6 +785,20 @@ export function ImageGenTab() { Keeps the system awake while reducing WindowServer GPU contention on affected Apple silicon. Turn off only when another headless workflow manages display power. + fal.ai API keyEnables the fal.ai queue video backend on the Video Gen page and in FableLoom. Get a key at fal.ai/dashboard/keys, or set the FAL_KEY environment variable instead.} + labelClassName="text-sm text-gray-300" + > + setFalApiKey(e.target.value)} + placeholder="fal-key-..." + className="w-full bg-port-bg border border-port-border rounded-lg px-3 py-2 text-sm text-white focus:outline-none focus:border-port-accent" + /> +
{RENDER_TARGET_OPTIONS.map(({ id, label, video }) => { const entry = renderDefaults[id] || {}; diff --git a/client/src/components/settings/ImageGenTab.test.jsx b/client/src/components/settings/ImageGenTab.test.jsx index 1644e91f04..05b9588692 100644 --- a/client/src/components/settings/ImageGenTab.test.jsx +++ b/client/src/components/settings/ImageGenTab.test.jsx @@ -244,7 +244,9 @@ describe('ImageGenTab grouped tabs', () => { fireEvent.click(screen.getByRole('button', { name: /^Save$/ })); await waitFor(() => expect(updateSettings).toHaveBeenCalled()); const patch = updateSettings.mock.calls[0][0]; - expect(patch.videoGen).toEqual({ mode: 'local', defaultModelId: 'ltx23_distilled_q4', displaySleep: true }); + expect(patch.videoGen).toEqual({ + mode: 'local', defaultModelId: 'ltx23_distilled_q4', displaySleep: true, fal: { apiKey: '' }, + }); }); }); diff --git a/client/src/hooks/useVideoGenFieldState.js b/client/src/hooks/useVideoGenFieldState.js index 479e07514e..c8922517da 100644 --- a/client/src/hooks/useVideoGenFieldState.js +++ b/client/src/hooks/useVideoGenFieldState.js @@ -21,6 +21,8 @@ export function useVideoGenFieldState({ }) { const [backend, setBackend] = useState('local'); const [grokDuration, setGrokDuration] = useState(GROK_VIDEO_DEFAULT_DURATION); + const [falDuration, setFalDuration] = useState(''); + const [falModelId, setFalModelId] = useState(''); const [mode, setMode] = useState(incomingAudioFilename ? 'a2v' : (incomingSourceImage ? 'image' : 'text')); const [prompt, setPrompt] = useState(incomingPrompt || ''); const [negativePrompt, setNegativePrompt] = useState(incomingNegativePrompt || ''); @@ -82,6 +84,8 @@ export function useVideoGenFieldState({ extendFromVideoId, setExtendFromVideoId, fps, setFps, grokDuration, setGrokDuration, + falDuration, setFalDuration, + falModelId, setFalModelId, guidanceScale, setGuidanceScale, height, setHeight, i2vReferenceMode, setI2vReferenceMode, diff --git a/client/src/hooks/useVideoGenForm.js b/client/src/hooks/useVideoGenForm.js index 79be036303..3d13904e8f 100644 --- a/client/src/hooks/useVideoGenForm.js +++ b/client/src/hooks/useVideoGenForm.js @@ -71,7 +71,9 @@ const editableRemixModel = (models, defaultModelId) => { * wire accepts — kept here rather than in the page so there stays exactly * one builder for what `server/routes/videoGen.js` validates. */ -export function useVideoGenForm({ models, modelContext, availableLoras, grokEnabled, remoteSubmissionFields = null }) { +export function useVideoGenForm({ + models, modelContext, availableLoras, grokEnabled, falEnabled = false, remoteSubmissionFields = null, +}) { const [searchParams, setSearchParams] = useSearchParams(); const incomingSourceImage = searchParams.get('sourceImageFile'); const incomingAudioFilename = searchParams.get('audioFilename'); @@ -92,6 +94,8 @@ export function useVideoGenForm({ models, modelContext, availableLoras, grokEnab extendFromVideoId, setExtendFromVideoId, fps, setFps, grokDuration, setGrokDuration, + falDuration, setFalDuration, + falModelId, setFalModelId, guidanceScale, setGuidanceScale, height, setHeight, i2vReferenceMode, setI2vReferenceMode, @@ -586,7 +590,12 @@ export function useVideoGenForm({ models, modelContext, availableLoras, grokEnab // lane reads only prompt/dims/source-image/duration, so its image_to_video // always anchors and the promise has to collapse to the default there. const isGrok = grokEnabled && backend === 'grok'; - const referenceModeApplies = mode === 'image' && !isGrok; + // fal.ai's queue REST API is usability-gated on a configured API key + // (`falEnabled`, mirrored from the server's isVideoModeUsable check), not a + // toggle — same short-circuit shape as grok: only prompt/dims/source-image + // and a duration reach the provider. + const isFal = falEnabled && backend === 'fal'; + const referenceModeApplies = mode === 'image' && !isGrok && !isFal; // The strength the render will actually use, for the slider readout — an // untouched slider under Inspire still resolves to the contract's low default // rather than the pipeline's 1.0, and the panel must say so. @@ -870,7 +879,7 @@ export function useVideoGenForm({ models, modelContext, availableLoras, grokEnab // clip survives the switch and reappears if the user flips back. const handleBackendChange = (id) => { setBackend(id); - if (id === 'grok' && mode !== 'text' && mode !== 'image') { + if ((id === 'grok' || id === 'fal') && mode !== 'text' && mode !== 'image') { handleModeChange((sourceImageFile || sourceImageUpload) ? 'image' : 'text'); } }; @@ -1160,6 +1169,12 @@ export function useVideoGenForm({ models, modelContext, availableLoras, grokEnab setBackend('grok'); setMode(p.videoMode === 'image' ? 'image' : 'text'); if (p.duration) setGrokDuration(p.duration); + } else if (p.mode === 'fal') { + // fal.ai job: same discriminator shape as grok above. + setBackend('fal'); + setMode(p.videoMode === 'image' ? 'image' : 'text'); + if (p.duration) setFalDuration(p.duration); + if (p.modelId) setFalModelId(p.modelId); } else if (p.mode) setMode(p.mode); if (p.chunks && p.chunks > 1) setChunks(p.chunks); // 0 is a real restored value ("last frame only"), so this can't gate on @@ -1232,7 +1247,7 @@ export function useVideoGenForm({ models, modelContext, availableLoras, grokEnab // Snapshot the current validated state into a wire payload. The submit flow // stays pure so all three backend contracts can be tested independently. const submissionState = { - isGrok, grokDuration, remoteSubmissionFields, + isGrok, grokDuration, isFal, falDuration, falModelId, remoteSubmissionFields, prompt, negativePrompt, stylePreset, selectedUniverse, width, height, mode, sourceImageFile, sourceImageUpload, numFrames, fps, steps, guidanceScale, seed, @@ -1248,8 +1263,10 @@ export function useVideoGenForm({ models, modelContext, availableLoras, grokEnab return { // Backend + mode - backend, isGrok, handleBackendChange, + backend, isGrok, isFal, handleBackendChange, grokDuration, setGrokDuration, + falDuration, setFalDuration, + falModelId, setFalModelId, mode, handleModeChange, // Prompt + style prompt, setPrompt, diff --git a/client/src/lib/imageGenModes.js b/client/src/lib/imageGenModes.js index 61b05aea0d..6a87d0bd4b 100644 --- a/client/src/lib/imageGenModes.js +++ b/client/src/lib/imageGenModes.js @@ -71,7 +71,7 @@ export const RENDER_TARGET_OPTIONS = Object.freeze([ // Client mirror of the server's VIDEO_GEN_MODES (services/videoGen/modes.js) — // the backend alphabet for the video pin controls above and the install-wide // `settings.videoGen.mode` pin. -export const VIDEO_RENDER_MODES = Object.freeze(['local', 'grok']); +export const VIDEO_RENDER_MODES = Object.freeze(['local', 'grok', 'fal']); // Client mirror of the server's normalizeRenderPinValue // (server/lib/renderTargets.js) — THE one render-pin normalization rule: trim; @@ -94,6 +94,7 @@ export const MODE_LABELS = Object.freeze({ [IMAGE_GEN_MODE.GROK]: 'Grok', [IMAGE_GEN_MODE.AGY]: 'Agy', [IMAGE_GEN_MODE.EXTERNAL]: 'External', + fal: 'fal.ai', }); // Client mirror of the server's CLOUD_IMAGE_GEN_MODES (imageGen/modes.js) — diff --git a/client/src/lib/videoGenSubmission.js b/client/src/lib/videoGenSubmission.js index 2c85fa6955..e454c778ab 100644 --- a/client/src/lib/videoGenSubmission.js +++ b/client/src/lib/videoGenSubmission.js @@ -34,7 +34,7 @@ export function envelopVideoPrompt(text, { } export function buildVideoGenSubmission({ - isGrok, grokDuration, remoteSubmissionFields, + isGrok, grokDuration, isFal, falDuration, falModelId, remoteSubmissionFields, prompt, negativePrompt, stylePreset, selectedUniverse, width, height, mode, sourceImageFile, sourceImageUpload, numFrames, fps, steps, guidanceScale, seed, @@ -71,6 +71,21 @@ export function buildVideoGenSubmission({ }; } + if (isFal) { + return { + backend: 'fal', + prompt: composed.prompt, + negativePrompt: composed.negativePrompt, + falDuration, + falModelId: falModelId || undefined, + width: clampImageEdge(width, VIDEO_EDGE_BOUNDS), + height: clampImageEdge(height, VIDEO_EDGE_BOUNDS), + mode: mode === 'image' ? 'image' : 'text', + sourceImageFile: mode === 'image' ? (sourceImageFile || '') : '', + sourceImage: mode === 'image' ? (sourceImageUpload || '') : '', + }; + } + if (remoteSubmissionFields) { return { backend: 'local', diff --git a/client/src/pages/VideoGen.jsx b/client/src/pages/VideoGen.jsx index 8e7aa73b41..b3054d6b28 100644 --- a/client/src/pages/VideoGen.jsx +++ b/client/src/pages/VideoGen.jsx @@ -134,13 +134,19 @@ export default function VideoGen() { // user enabled Grok in Settings → Image Gen (one toggle covers image + // video). 'local' keeps every existing flow untouched. const [grokEnabled, setGrokEnabled] = useState(false); + // fal.ai queue REST video backend (#6213) — surfaced only when an API key is + // configured (Settings → Video Gen, or the FAL_KEY env var). + const [falEnabled, setFalEnabled] = useState(false); // The jobId of the render this tab's Generate button currently owns — // threaded into cancelVideoGen so cancellation is job-scoped. const activeJobIdRef = useRef(null); const models = useMemo(() => modelContext?.models || [], [modelContext]); const refreshGrokEnabled = useCallback(() => { getSettings({ silent: true }) - .then((sv) => setGrokEnabled(sv?.imageGen?.grok?.enabled === true)) + .then((sv) => { + setGrokEnabled(sv?.imageGen?.grok?.enabled === true); + setFalEnabled(Boolean(sv?.videoGen?.fal?.apiKey)); + }) .catch(() => {}); }, []); useEffect(() => { refreshGrokEnabled(); }, [refreshGrokEnabled]); @@ -157,7 +163,8 @@ export default function VideoGen() { // Every field the form submits, plus the payload builder both submit paths // share. See client/src/hooks/useVideoGenForm.js. const { - backend, isGrok, handleBackendChange, grokDuration, setGrokDuration, + backend, isGrok, isFal, handleBackendChange, grokDuration, setGrokDuration, + falDuration, setFalDuration, falModelId, setFalModelId, mode, handleModeChange, prompt, setPrompt, envelopedPrompt, negativePrompt, setNegativePrompt, stylePreset, setStylePreset, selectedUniverse, setSelectedUniverse, remixModelFallback, @@ -192,7 +199,7 @@ export default function VideoGen() { icStrength, setIcStrength, icSkipStage2, setIcSkipStage2, applyRemix, applyFinish, applyResumedParams, buildGeneratePayload, } = useVideoGenForm({ - models, modelContext, availableLoras, grokEnabled, + models, modelContext, availableLoras, grokEnabled, falEnabled, remoteSubmissionFields: remoteTarget.isRemote ? remoteTarget.submissionFields : null, }); @@ -216,6 +223,7 @@ export default function VideoGen() { const model = remoteTarget.model; const present = [ ['the Grok backend', isGrok], + ['the fal.ai backend', isFal], // Each remaining pipeline semantic has its own input listed below, but the // mode can be set before that input is filled — so gate the mode too // rather than letting an a2v render reach the peer as plain text-to-video. @@ -242,7 +250,7 @@ export default function VideoGen() { return `${model?.modelName || 'The selected peer model'} renders only from a source image — add a start frame, or pick a text-to-video model.`; } return null; - }, [remoteTarget.isRemote, remoteTarget.model, remoteTarget.acceptsInput, isGrok, mode, sourceImageFile, sourceImageUpload, + }, [remoteTarget.isRemote, remoteTarget.model, remoteTarget.acceptsInput, isGrok, isFal, mode, sourceImageFile, sourceImageUpload, lastImageFile, lastImageUpload, keyframesActive, extendFromVideoId, audioFile, icReferenceFile, icReferenceVideoId, icReferenceImageFiles, selectedLoras, chunks]); // One reading for the Generate button, the enqueue guard and the caption. @@ -606,14 +614,14 @@ export default function VideoGen() { startEncoderWhenIdle(option && !option.builtIn ? textEncoderDownloadId(id) : null); }, [setTextEncoderId, textEncoderOptions, startEncoderWhenIdle]); const icWeightStatus = icSpec ? modelDownload.getStatus(icSpec.mode) : null; - const modelWeightsBlocked = !isGrok + const modelWeightsBlocked = !isGrok && !isFal && (statusLoading || !modelId || !currentModel || modelDownload.loading || modelStatus === null || modelStatus?.cached === false); - const textEncoderWeightsBlocked = !isGrok && usesSharedTextEncoder + const textEncoderWeightsBlocked = !isGrok && !isFal && usesSharedTextEncoder && (modelDownload.loading || textEncoderStatus === null || textEncoderStatus?.cached === false); - const icWeightsBlocked = !isGrok && icModeActive + const icWeightsBlocked = !isGrok && !isFal && icModeActive && (modelDownload.loading || icWeightStatus === null || icWeightStatus?.cached === false); - const textEncoderOptionBlocked = !isGrok && !!textEncoderOptionDownloadId + const textEncoderOptionBlocked = !isGrok && !isFal && !!textEncoderOptionDownloadId && (modelDownload.loading || textEncoderOptionStatus === null || textEncoderOptionStatus?.cached === false); const weightsGateBlocked = modelWeightsBlocked || textEncoderWeightsBlocked || textEncoderOptionBlocked || icWeightsBlocked; @@ -686,7 +694,7 @@ export default function VideoGen() { // actually apply it (macOS, and the user hasn't opted out). const rendersSleepDisplay = !!status?.displaySleepOnRender && !!currentModel?.sleepsDisplayDuringRender - && !remoteTarget.isRemote && !isGrok; + && !remoteTarget.isRemote && !isGrok && !isFal; // Run a single payload through the SSE pipeline. Returns a promise that // resolves when the job completes (or rejects on error / cancel). The @@ -836,7 +844,7 @@ export default function VideoGen() { // will actually run on. const canEnqueue = prompt.trim() && (remoteTarget.isRemote ? remoteBlocked === null - : (isGrok || (!notConnected && !extendModeBlocked + : (isGrok || isFal || (!notConnected && !extendModeBlocked && !a2vModeBlocked && !icLoraModeBlocked && !byovGateBlocked && !weightsGateBlocked && !keyframesBlocked))); @@ -911,12 +919,16 @@ export default function VideoGen() { })()} {/* Backend switch — shown only when the user enabled Grok in Settings → - Image Gen. Grok's image_to_video supports text (image-first) and - image modes only, so switching to it snaps an unsupported mode back - to the nearest one. */} - {grokEnabled && ( + Image Gen and/or configured a fal.ai API key. Both cloud backends' + image-to-video only supports text (image-first) and image modes, so + switching to either snaps an unsupported mode back to the nearest one. */} + {(grokEnabled || falEnabled) && (
- {[{ id: 'local', label: 'Local' }, { id: 'grok', label: 'Grok' }].map(({ id, label }) => ( + {[ + { id: 'local', label: 'Local' }, + ...(grokEnabled ? [{ id: 'grok', label: 'Grok' }] : []), + ...(falEnabled ? [{ id: 'fal', label: 'fal.ai' }] : []), + ].map(({ id, label }) => ( @@ -939,7 +955,7 @@ export default function VideoGen() { WAI-ARIA Tabs, since the mode-specific inputs aren't structured as tabpanels and we don't implement roving-tabindex/arrow-key focus. */}
- {(isGrok ? MODES.filter((m) => m.id === 'text' || m.id === 'image') : MODES).map(({ id, label, icon: Icon, desc }) => { + {((isGrok || isFal) ? MODES.filter((m) => m.id === 'text' || m.id === 'image') : MODES).map(({ id, label, icon: Icon, desc }) => { const active = mode === id; return ( @@ -955,7 +965,7 @@ export default function VideoGen() { WAI-ARIA Tabs, since the mode-specific inputs aren't structured as tabpanels and we don't implement roving-tabindex/arrow-key focus. */}
- {((isGrok || isFal) ? MODES.filter((m) => m.id === 'text' || m.id === 'image') : MODES).map(({ id, label, icon: Icon, desc }) => { + {((isGrok || isFal || isReactor) ? MODES.filter((m) => m.id === 'text' || m.id === 'image') : MODES).map(({ id, label, icon: Icon, desc }) => { const active = mode === id; return ( +
+ {entries.map((entry, i) => ( +
+ update(i, { id: e.target.value })} + placeholder={idPlaceholder} aria-label={`${label} id`} + className="w-28 shrink-0 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent" + /> + update(i, { descriptor: e.target.value })} + placeholder="byte-stable visual descriptor" aria-label={`${label} descriptor`} + className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent" + /> + +
+ ))} +
+ ); +} + +function SceneEditor({ scene, onChange, onRemove, busy }) { + const updateLine = (i, patch) => onChange({ ...scene, lines: scene.lines.map((l, idx) => (idx === i ? { ...l, ...patch } : l)) }); + const addLine = () => onChange({ ...scene, lines: [...scene.lines, emptyLine()] }); + const removeLine = (i) => onChange({ ...scene, lines: scene.lines.filter((_, idx) => idx !== i) }); + return ( +
+
+ + onChange({ ...scene, location: e.target.value })} + disabled={busy} placeholder="location id (matches a Locations bible entry)" + className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent disabled:opacity-50" + /> + +
+
+ {scene.lines.map((line, i) => ( +
+ + {line.type === 'dialogue' && ( + updateLine(i, { speaker: e.target.value })} + disabled={busy} placeholder="speaker id" aria-label="Speaker id" + className="w-24 shrink-0 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent disabled:opacity-50" + /> + )} + updateLine(i, { text: e.target.value })} + disabled={busy} placeholder={line.type === 'dialogue' ? 'what they say' : 'what happens'} aria-label="Line text" + className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent disabled:opacity-50" + /> + +
+ ))} +
+ +
+ ); +} + +function ClipPreviewCard({ clip, result, framing, onFramingChange, busy }) { + const pass = result?.pass !== false; + return ( +
+
+
+ #{result?.index ?? 0} + + {clip.cutType === 'continue' ? 'continue' : 'fresh cut'} + + {clip.speakers?.map((s) => ( + {s} + ))} +
+ {pass ? ( + + ) : ( + + )} +
+

{clip.prompt}

+ {clip.references?.length > 0 && ( +
+ slot locks: + {clip.references.map((r) => {r.kind}/{r.id})} +
+ )} + {clip.cutType === 'continue' && ( +
+ + onFramingChange(e.target.value)} + placeholder="e.g. close-up on Mara's face" + className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-[11px] text-white focus:outline-none focus:border-port-accent disabled:opacity-50" + /> +
+ )} + {!pass && ( +
    + {result.reasons.map((r) =>
  • {r}
    • )} +
+ )} +
+ ); +} + +export default function EpisodeComposer({ initialScenes, onQueued }) { + const [scenes, setScenes] = useState(() => (initialScenes?.length ? initialScenes.map((s) => ({ + key: uuidv4(), sceneId: s.sceneId || uuidv4(), location: s.location || '', + lines: s.lines.map((l) => ({ key: uuidv4(), type: l.type, speaker: l.speaker || '', voice: l.voice || '', text: l.text || '' })), + })) : [emptyScene()])); + const [styleDescriptor, setStyleDescriptor] = useState(''); + const [castEntries, setCastEntries] = useState([]); + const [locationEntries, setLocationEntries] = useState([]); + const [backend, setBackend] = useState('local'); + const [framings, setFramings] = useState({}); // { [`${sceneIndex}:${beatIndex}`]: string } + const [preview, setPreview] = useState(null); // { clips, lint } + const [previewLoading, setPreviewLoading] = useState(false); + const [queuedJobId, setQueuedJobId] = useState(null); + const debounceRef = useRef(null); + + const bible = useMemo(() => buildBible(styleDescriptor, castEntries, locationEntries), [styleDescriptor, castEntries, locationEntries]); + const sanitizedScenes = useMemo(() => sanitizeScenesForRequest(scenes), [scenes]); + const hasContent = sanitizedScenes.length > 0; + + const sse = useSseProgress(queuedJobId ? continuousVideoEpisodeEventsUrl(queuedJobId) : null); + + // Debounced live preview — recompiles + relints on every scene/bible/framing + // edit. Skipped once queued: the composer is read-only during generation. + // `sanitizedScenes`/`bible` are already fresh-only-on-change via useMemo, so + // depending on them directly (no JSON.stringify) is both cheaper and correct. + useEffect(() => { + if (queuedJobId) return undefined; + if (!hasContent) { setPreview(null); setPreviewLoading(false); return undefined; } + setPreviewLoading(true); + debounceRef.current = setTimeout(async () => { + const result = await lintContinuousVideoEpisode({ + scenes: sanitizedScenes, bible, framings: framingsArrayFor(preview?.clips, framings), + }).catch(() => null); + setPreview(result); + setPreviewLoading(false); + }, PREVIEW_DEBOUNCE_MS); + return () => clearTimeout(debounceRef.current); + // Re-runs on any scene/bible/framing edit; `preview` itself is excluded to + // avoid retriggering off its own write. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [sanitizedScenes, bible, framings, queuedJobId]); + + useEffect(() => { + if (sse.latest?.type === 'complete') { + toast.success('Episode generated'); + onQueued?.(sse.latest.result); + } else if (sse.latest?.type === 'error') { + toast.error(sse.latest.error?.message || sse.latest.error || 'Episode generation failed'); + } + }, [sse.latest, onQueued]); + + const [handleQueue, queuing] = useAsyncAction(async () => { + const result = await generateContinuousVideoEpisode({ + scenes: sanitizedScenes, bible, framings: framingsArrayFor(preview.clips, framings), backend, + }, { silent: true }); // useAsyncAction owns the error toast + setQueuedJobId(result.jobId); + }, { errorMessage: 'Failed to queue episode' }); + + const canQueue = hasContent && preview?.lint?.pass === true && !queuing && !queuedJobId; + + const resetForNewEpisode = () => { + setQueuedJobId(null); + setScenes([emptyScene()]); + setFramings({}); + setPreview(null); + }; + + const streaming = !!queuedJobId && !sse.closed; + const busy = queuing || streaming; + + return ( +
+
+ + setStyleDescriptor(e.target.value)} + placeholder="e.g. gritty 35mm noir, high contrast" + className="w-full bg-port-bg border border-port-border rounded px-2 py-1.5 text-xs text-white focus:outline-none focus:border-port-accent disabled:opacity-50" + /> +
+ + + + +
+
+ Scenes + +
+ {scenes.map((scene, i) => ( + setScenes((prev) => prev.map((s, idx) => (idx === i ? next : s)))} + onRemove={() => setScenes((prev) => prev.filter((_, idx) => idx !== i))} + /> + ))} +
+ +
+ + +
+ +
+
+ Beat preview + {previewLoading && } +
+ {!hasContent &&

Write at least one scene to see a beat preview.

} + {preview?.clips?.map((clip, i) => ( + setFramings((prev) => ({ ...prev, [clipFramingKey(clip)]: v }))} + /> + ))} +
+ + {queuedJobId ? ( +
+
+ + {streaming ? (sse.latest?.message || 'Generating…') : 'Episode generation finished'} +
+ {typeof sse.latest?.progress === 'number' && ( +
+
+
+ )} + {!streaming && ( + + )} +
+ ) : ( + + )} +
+ ); +} diff --git a/client/src/components/videoGen/EpisodeComposer.test.jsx b/client/src/components/videoGen/EpisodeComposer.test.jsx new file mode 100644 index 0000000000..1c7d164964 --- /dev/null +++ b/client/src/components/videoGen/EpisodeComposer.test.jsx @@ -0,0 +1,90 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; +import EpisodeComposer from './EpisodeComposer'; +import { MockEventSource, lastEventSource } from '../../test/mockEventSource'; + +vi.mock('../../services/api', () => ({ + lintContinuousVideoEpisode: vi.fn(), + generateContinuousVideoEpisode: vi.fn(), + continuousVideoEpisodeEventsUrl: (jobId) => `/api/continuous-video/${jobId}/events`, +})); + +import { + lintContinuousVideoEpisode, generateContinuousVideoEpisode, +} from '../../services/api'; + +const FAILING_PREVIEW = { + clips: [{ + cutType: 'fresh', speakers: ['mara'], prompt: 'Mara paces.', references: [{ kind: 'cast', id: 'mara' }], + }], + lint: { pass: false, results: [{ index: 0, pass: false, reasons: ['no bible descriptor found for cast/mara'] }] }, +}; + +const PASSING_PREVIEW = { + clips: [{ + cutType: 'fresh', speakers: ['mara'], prompt: 'gritty noir. a cell. Mara paces.', references: [{ kind: 'cast', id: 'mara' }], + }], + lint: { pass: true, results: [{ index: 0, pass: true, reasons: [] }] }, +}; + +async function writeScene() { + const [textInput] = screen.getAllByPlaceholderText('what happens'); + fireEvent.change(textInput, { target: { value: 'Mara paces the cell.' } }); +} + +describe('EpisodeComposer', () => { + beforeEach(() => { + vi.clearAllMocks(); + MockEventSource.reset(); + global.EventSource = MockEventSource; + }); + afterEach(() => { + delete global.EventSource; + }); + + it('renders a beat preview with camera-cut and slot-lock info once scenes have content', async () => { + lintContinuousVideoEpisode.mockResolvedValue(PASSING_PREVIEW); + render(); + await writeScene(); + await waitFor(() => expect(lintContinuousVideoEpisode).toHaveBeenCalled(), { timeout: 2000 }); + expect(await screen.findByText('fresh cut')).toBeTruthy(); + expect(screen.getByText('mara')).toBeTruthy(); + expect(screen.getByText('cast/mara')).toBeTruthy(); + }); + + it('shows lint failure reasons for a failing beat and keeps Queue disabled', async () => { + lintContinuousVideoEpisode.mockResolvedValue(FAILING_PREVIEW); + render(); + await writeScene(); + await waitFor(() => expect(lintContinuousVideoEpisode).toHaveBeenCalled(), { timeout: 2000 }); + expect(await screen.findByText(/no bible descriptor found for cast\/mara/)).toBeTruthy(); + expect(screen.getByRole('button', { name: /Queue episode/i })).toBeDisabled(); + }); + + it('gates Queue episode on a passing lint, then streams progress and disables the form while queuing', async () => { + lintContinuousVideoEpisode.mockResolvedValue(PASSING_PREVIEW); + generateContinuousVideoEpisode.mockResolvedValue({ jobId: 'job-1', generationId: 'job-1', status: 'running' }); + render(); + await writeScene(); + await waitFor(() => expect(lintContinuousVideoEpisode).toHaveBeenCalled(), { timeout: 2000 }); + + const queueButton = await screen.findByRole('button', { name: /Queue episode/i }); + await waitFor(() => expect(queueButton).not.toBeDisabled()); + fireEvent.click(queueButton); + + await waitFor(() => expect(generateContinuousVideoEpisode).toHaveBeenCalled()); + await waitFor(() => expect(lastEventSource()).toBeTruthy()); + + // Streaming — the scene text input is disabled (save-gating: no editing mid-generation). + await waitFor(() => { + const [textInput] = screen.getAllByPlaceholderText('what happens'); + expect(textInput.disabled).toBe(true); + }); + + act(() => lastEventSource().emit({ type: 'progress', progress: 0.5, message: 'Rendering clip 1/1' })); + expect(await screen.findByText('Rendering clip 1/1')).toBeTruthy(); + + act(() => lastEventSource().emit({ type: 'complete', result: { jobId: 'job-1' } })); + expect(await screen.findByText(/Compose another episode/i)).toBeTruthy(); + }); +}); diff --git a/client/src/lib/README.md b/client/src/lib/README.md index d7e1cd4fe3..e617d6fee8 100644 --- a/client/src/lib/README.md +++ b/client/src/lib/README.md @@ -168,6 +168,7 @@ grep -i "what you want to do" client/src/lib/README.md | `universeBuilderExpand.js` | `mergeExpandIntoDraft(draft, result)` — pure merge of a Universe Builder draft with the LLM expand-API response (lock honoring, category/sheet merge with `kind` precedence, canon dedupe by name/slugline/alias). Also exports `mergeVariations`, `mergeCanonByName`, and `extractPreservedFromDraft` for callers that need the building blocks (per-category Generate, save-time refetch+merge). | | `editorialChecks.js` | Pure helpers for the Editorial Checks page (`/pipeline/editorial-checks`, #1285): `groupChecksByScope(checks)` buckets catalog rows into ordered Noun/Scene/Issue/Series sections; `groupFindingsByCheck(comments, rowsById)` groups check-sourced review comments per check with open/total + severity counts; `openFindingsTotal(groups)` sums open findings; `findingManuscriptLink(seriesId, comment)` builds the `?comment=` deep-link into the manuscript editor; the triage filter/sort helpers (#1600) `deriveFindingFacets(groups)` (enumerate present severities/statuses/scopes/checks/issues), `applyFindingsView(groups, filters, sort)` (filter + recount + sort findings and groups), `normalizeFindingSort`, `findingIssueKey`, and the `FINDING_SORT_OPTIONS` table; the per-check maturity/quality helpers (#1629) `checkDismissalRate(group)` and `checkMaturity(group)` (→ `{ findings, dismissalRate, falsePositiveRate, level }`, `level` ∈ new/unproven/noisy/reliable) plus the `CHECK_MATURITY_MIN_SAMPLE`/`CHECK_NOISY_DISMISSAL_RATE` thresholds; plus `scopeLabel` and the `CHECK_SCOPE_ORDER`/`CHECK_SCOPE_LABELS` tables. No React/window — the page and its tests share them. | | `editorialHealth.js` | Pure presentation helpers for the editorial health panel (`/pipeline/editorial-checks`, #1316): `scoreBand(score)` bands a 0–100 health score into a label + Tailwind tone; `deltaDisplay(delta)` formats the revision-to-revision score change (arrow + tone); `sparklineGeometry(points, box)` projects the trend's score points into SVG polyline coords; `orderedCategories(openByCategory)` sorts the per-category open-finding breakdown; `orderedChecks(openByCheck, labelFor)` sorts the per-check breakdown resolving each id to its catalog label (#1597); `checkCountSeries(points, checkId)` extracts one check's count series across the trend points; `countSparklineGeometry(values, box)` projects a non-negative count series into SVG polyline coords normalized to its own max (per-check finding-count sparkline); `diffCountMaps(current, previous)` lists the buckets whose open-finding count changed between two snapshots (#1630); `snapshotDiff(point, previous)` builds a trend snapshot's drill-down (its own breakdown + per-severity/category/check diff vs the prior revision, null-aware for pre-#1597 per-check telemetry, #1630); plus the `SEVERITY_ORDER`/`SEVERITY_LABELS` and `READINESS_GATE_LABELS`/`READINESS_GATE_ORDER` tables. No React/window — the panel and its tests share them. | +| `episodeSceneImport.js` | Best-effort mapping of a FableLoom episode's scene nodes into the Episode Composer's draft `scenes` shape (#6228): `parseNodeTextToLines(text, { format })` splits prose into a single action line, or (for `teleplay`) picks out ALL-CAPS character cues + following dialogue lines and drops sluglines; `loomEpisodeToDraftScenes(episode, { format })` maps every node to one draft scene. Lossy on purpose — the composer always leaves the import user-editable. | | `seriesReviewProgress.js` | Pure helpers for the "Review this series" SSE progress stream (`SeriesReviewPanel.jsx`, #4108): `REVIEW_STEP_LABELS` + `reviewFrameLabel(frame)` label a single frame, and `summarizeReviewProgress(frames)` folds the whole stream into `{ headline, alsoRunning }`. Since the review runs the foundation judge and canon readiness concurrently with the editorial-checks pass, frames interleave — labelling only the newest frame made the headline flicker between a background step that just settled and the current check. The summarizer tracks which steps are still in flight, gives the headline to the editorial-checks pass while it runs, and reports the other in-flight steps alongside it. No React/window. | | `drumKits.js` | The three selectable synth kits for the SongBook drum play-along — `DRUM_KIT_LIST` (the one ordered source of truth, first entry = default; the picker renders it directly) plus the derived `DRUM_KITS` / `DRUM_KIT_IDS` / `DEFAULT_DRUM_KIT` (`909` punchy, `808` deep-sub, `acoustic`), `resolveDrumKit(id)` (unknown/stale id → the default, never silence), `kitVoiceLayers(kit, sound)` (unmapped piece → the snare, never silence) and the kit-independent `CLICK_VOICE`. Pure DATA: each voice is a list of LAYERS — `tone` (`wave, from, to, pitchDecay, drive`), `noise` (`filter`), or `metal` (N square `partials` summed into one filter+envelope) — that `drumPlayback.js` realizes in Web Audio. Three properties carry the kit's punch and are easy to undo by accident: `pitchDecay` is INDEPENDENT of the amplitude `decay` (a membrane snaps to its fundamental in ~30–60ms then holds while the tail runs on — coupling the two is what made the original kick sound weak), `drive` tanh-saturates a layer so a sub-50 Hz kick generates the harmonics a phone/laptop speaker can actually reproduce, and a `metal` layer's high-pass cutoffs are far LOWER (and gains far higher) than the noise voices beside them because a square's harmonics fall off 6 dB/octave — see the calibration warning in the file before "harmonizing" those numbers. | | `drumNotation.js` | Parser for the PortOS drum-kit notation (#3115) — a dependency-free GRID DSL (`HH: x x x x`) for the SongBook `drum` content format, drawn by `` (hand-rolled SVG, no engraving lib). `parseDrumChart(text)` → `{ time, tempo, subdivision, stepsPerBar, bars:[{index,label,repeat,repeatPass,rows:[{piece,cells}]}], pieces, errors }` — `key: value` headers (`time`/`tempo`/`subdivision`/`kit` row-order override), blank-line-separated bar blocks with a `# label x2` repeat suffix expanded into real bars, nine kit pieces (`CR RD HH T1 T2 S FT K HF`, long aliases accepted), six cell glyphs (`-` rest, `x` normal, `X` accent, `o` open, `g` ghost, `f` flam) with spacing optional. Forgiving: short rows pad with rests, over-long rows truncate, unknown pieces/cells collect into `errors` — never throws. Plus `KIT_PIECES` (label / GM `midi` / synth `sound` / `glyph`), `kitPiece(id)`, `CELL_GLYPHS` (per-glyph velocity + flags), `isDrumNotation(text)` (import-page format sniff, runs BEFORE `tabNotation.detectFormat`), `drumChartHasMusic(text)` and its already-parsed twin `chartHasMusic(chart)`. Also the plain-language explainers the sheet's tap-a-note readout and Legend panel read — `describeDrumCell(pieceId, cell)` → `{ pieceLabel, char, articulation, detail, technique, velocityPercent, rest }` (piece-aware: `o` is "Open" on a hi-hat/cymbal but a normal hit on a drum; a null/unknown cell describes as a rest, an unknown piece returns null), `describeDrumPosition(bar, step, subdivision)` → `bar 6, count “2 e”` (count-along syllables, exact `+n/N` fallback for an unconventional subdivision), `drumGlyphLegend(pieceIds)` (all six cell characters, with the `o` row resolved for the pieces a chart uses — a fixed row would tell a crash-only chart to work the hi-hat pedal; a mixed kit gets every applicable reading) and each `KIT_PIECES` row's `technique` (how that piece is struck). Only the hi-hat voice sustains an `o` in playback, so the `open` wording follows the piece's `sound`, not its glyph shape. NOT the lead-sheet parser (`scoreNotation.js`). | diff --git a/client/src/lib/episodeSceneImport.js b/client/src/lib/episodeSceneImport.js new file mode 100644 index 0000000000..ebe1392dd2 --- /dev/null +++ b/client/src/lib/episodeSceneImport.js @@ -0,0 +1,74 @@ +// Best-effort mapping from a FableLoom scene node's authored text into the +// `{ lines: [{ type, speaker, text }] }` shape /api/continuous-video expects +// (#6228). There is no existing FableLoom→continuous-video scene export, and +// prose/teleplay text is free-form — this is a lossy starting draft the +// Episode Composer always leaves user-editable, not a round-trip parser. + +const CUE_RE = /^[A-Z][A-Z0-9 .,'()-]{1,60}$/; + +/** True for a short all-caps line that reads as a teleplay character cue. */ +const looksLikeCue = (line) => { + const trimmed = line.trim(); + if (!trimmed || trimmed.length > 60) return false; + if (!CUE_RE.test(trimmed)) return false; + return /[A-Z]/.test(trimmed) && trimmed === trimmed.toUpperCase(); +}; + +/** + * Parse one FableLoom node's text into continuous-video scene lines. + * `format` 'teleplay' looks for ALL-CAPS character cues followed by dialogue; + * anything else (including 'prose') is imported as a single action line. + * Blank lines and slugline-looking lines (`INT. ...` / `EXT. ...`) are dropped. + */ +export function parseNodeTextToLines(text, { format = 'prose' } = {}) { + const raw = typeof text === 'string' ? text : ''; + if (!raw.trim()) return []; + const rawLines = raw.split('\n').map((l) => l.trim()).filter(Boolean); + + if (format !== 'teleplay') { + return [{ type: 'action', text: raw.trim() }]; + } + + const lines = []; + let actionBuffer = []; + const flushAction = () => { + if (actionBuffer.length) { + lines.push({ type: 'action', text: actionBuffer.join(' ') }); + actionBuffer = []; + } + }; + + for (let i = 0; i < rawLines.length; i += 1) { + const line = rawLines[i]; + if (/^(INT\.|EXT\.)/.test(line)) continue; // slugline — not a spoken/action line + if (looksLikeCue(line)) { + flushAction(); + const speaker = line.replace(/\(.*\)$/, '').trim(); + const next = rawLines[i + 1]; + if (next && !looksLikeCue(next) && !/^(INT\.|EXT\.)/.test(next)) { + lines.push({ type: 'dialogue', speaker, text: next }); + i += 1; + } + continue; + } + actionBuffer.push(line); + } + flushAction(); + return lines; +} + +/** + * Map a FableLoom loom's episode nodes into draft continuous-video scenes — + * one scene per node, in node order. Nodes with no importable lines are + * skipped. + */ +export function loomEpisodeToDraftScenes(episode, { format = 'prose' } = {}) { + const nodes = Array.isArray(episode?.nodes) ? episode.nodes : []; + return nodes + .map((node) => ({ + sceneId: node.id, + location: node.title || undefined, + lines: parseNodeTextToLines(node.prose, { format }), + })) + .filter((scene) => scene.lines.length > 0); +} diff --git a/client/src/lib/episodeSceneImport.test.js b/client/src/lib/episodeSceneImport.test.js new file mode 100644 index 0000000000..3cac8b3949 --- /dev/null +++ b/client/src/lib/episodeSceneImport.test.js @@ -0,0 +1,49 @@ +import { describe, it, expect } from 'vitest'; +import { parseNodeTextToLines, loomEpisodeToDraftScenes } from './episodeSceneImport.js'; + +describe('parseNodeTextToLines', () => { + it('imports prose as a single action line', () => { + expect(parseNodeTextToLines('You step into the hall. It is dark.', { format: 'prose' })).toEqual([ + { type: 'action', text: 'You step into the hall. It is dark.' }, + ]); + }); + + it('returns an empty array for blank/missing text', () => { + expect(parseNodeTextToLines('', { format: 'prose' })).toEqual([]); + expect(parseNodeTextToLines(undefined, { format: 'prose' })).toEqual([]); + }); + + it('extracts teleplay cues + dialogue and drops sluglines', () => { + const text = [ + 'INT. HOLDING CELL - NIGHT', + 'Mara paces the cell.', + 'MARA', + "We're out of time.", + 'She looks at the door.', + ].join('\n'); + expect(parseNodeTextToLines(text, { format: 'teleplay' })).toEqual([ + { type: 'action', text: 'Mara paces the cell.' }, + { type: 'dialogue', speaker: 'MARA', text: "We're out of time." }, + { type: 'action', text: 'She looks at the door.' }, + ]); + }); +}); + +describe('loomEpisodeToDraftScenes', () => { + it('maps each node to a scene and skips nodes with no importable lines', () => { + const episode = { + nodes: [ + { id: 'n1', title: 'Cell Block', prose: 'You wake up.' }, + { id: 'n2', title: 'Empty', prose: '' }, + ], + }; + expect(loomEpisodeToDraftScenes(episode)).toEqual([ + { sceneId: 'n1', location: 'Cell Block', lines: [{ type: 'action', text: 'You wake up.' }] }, + ]); + }); + + it('handles a missing/empty node list', () => { + expect(loomEpisodeToDraftScenes(null)).toEqual([]); + expect(loomEpisodeToDraftScenes({ nodes: [] })).toEqual([]); + }); +}); diff --git a/client/src/lib/index.js b/client/src/lib/index.js index a730dd9e5b..b379555d19 100644 --- a/client/src/lib/index.js +++ b/client/src/lib/index.js @@ -137,6 +137,7 @@ export * from './drumNotation.js'; export * from './drumPlayback.js'; export * from './editorialChecks.js'; export * from './editorialHealth.js'; +export * from './episodeSceneImport.js'; export * from './lookaheadTransport.js'; export * from './lossSparkline.js'; export * from './metronome.js'; diff --git a/client/src/pages/FableLoomStory.jsx b/client/src/pages/FableLoomStory.jsx index 9e55650756..535c1ab295 100644 --- a/client/src/pages/FableLoomStory.jsx +++ b/client/src/pages/FableLoomStory.jsx @@ -761,6 +761,7 @@ export default function FableLoomStory({ view = 'graph' }) { universe={linkedUniverse} onLoomUpdate={setLoom} onRewritten={handleRewritten} + episodeId={seriesPlanOpen ? null : episodeId} /> {episode && ( diff --git a/client/src/pages/VideoGen.jsx b/client/src/pages/VideoGen.jsx index e53c59e69e..2dab94fd38 100644 --- a/client/src/pages/VideoGen.jsx +++ b/client/src/pages/VideoGen.jsx @@ -54,6 +54,7 @@ import ModelDisclosure from '../components/videoGen/ModelDisclosure'; import ModelRepairBanner from '../components/videoGen/ModelRepairBanner'; import RenderStatusCard from '../components/videoGen/RenderStatusCard'; import VideoGenGallery from '../components/videoGen/VideoGenGallery'; +import EpisodeComposer from '../components/videoGen/EpisodeComposer'; import GalleryImagePicker from '../components/imageGen/GalleryImagePicker'; import MediaPreview from '../components/media/MediaPreview'; import StylePresetPicker from '../components/media/StylePresetPicker'; @@ -90,7 +91,9 @@ import { getSettings, getVideoGenRuntimeStatus, listLorasFull, + getLoom, } from '../services/api'; +import { loomEpisodeToDraftScenes } from '../lib/episodeSceneImport.js'; import LoraPicker from '../components/imageGen/LoraPicker'; import { VIDEO_RESOLUTIONS, resolutionOptionsForModel } from '../lib/videoGenResolutions'; import { GROK_VIDEO_DURATIONS } from '../lib/grokVideoClip.js'; @@ -120,6 +123,30 @@ export default function VideoGen() { refreshGrokEnabled(); }; + // Episode Composer (#6228) — multi-scene continuous-video authoring, opened + // either directly or as a FableLoom entry point carrying loomId/episodeId + // (LoomSettingsDrawer's "Open Episode Composer" link). + const episodeOpen = searchParams.get('episode') === '1'; + const episodeLoomId = searchParams.get('loomId'); + const episodeEpisodeId = searchParams.get('episodeId'); + const openEpisodeComposer = () => setSearchParams(prev => { const n = new URLSearchParams(prev); n.set('episode', '1'); return n; }); + const closeEpisodeComposer = () => setSearchParams(prev => { + const n = new URLSearchParams(prev); + n.delete('episode'); n.delete('loomId'); n.delete('episodeId'); + return n; + }); + const [episodeImportScenes, setEpisodeImportScenes] = useState(null); + useEffect(() => { + if (!episodeOpen || !episodeLoomId || !episodeEpisodeId) return; + getLoom(episodeLoomId, { silent: true }).then((loom) => { + const episode = loom?.episodes?.find((e) => e.id === episodeEpisodeId); + const draftScenes = loomEpisodeToDraftScenes(episode, { format: loom?.format }); + if (draftScenes.length) setEpisodeImportScenes(draftScenes); + }).catch(() => {}); + // Only re-run when a fresh loom/episode is targeted, not on every render. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [episodeOpen, episodeLoomId, episodeEpisodeId]); + // `/status` owns connectivity ONLY. It shells out to python on every call // (~1-2s), so nothing the form needs to render may wait on it. const [status, setStatus] = useState(null); @@ -887,6 +914,14 @@ export default function VideoGen() { > +
{entries.map((entry, i) => ( -
+
update(i, { id: e.target.value })} + type="text" value={entry.id} disabled={busy} onChange={(e) => update(i, { id: e.target.value })} placeholder={idPlaceholder} aria-label={`${label} id`} - className="w-28 shrink-0 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent" + className="w-28 shrink-0 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent disabled:opacity-50" /> update(i, { descriptor: e.target.value })} + type="text" value={entry.descriptor} disabled={busy} onChange={(e) => update(i, { descriptor: e.target.value })} placeholder="byte-stable visual descriptor" aria-label={`${label} descriptor`} - className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent" + className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-xs text-white focus:outline-none focus:border-port-accent disabled:opacity-50" /> -
@@ -197,11 +199,13 @@ function ClipPreviewCard({ clip, result, framing, onFramingChange, busy }) { ); } +const scenesFromImport = (imported) => imported.map((s) => ({ + key: uuidv4(), sceneId: s.sceneId || uuidv4(), location: s.location || '', + lines: s.lines.map((l) => ({ key: uuidv4(), type: l.type, speaker: l.speaker || '', voice: l.voice || '', text: l.text || '' })), +})); + export default function EpisodeComposer({ initialScenes, onQueued }) { - const [scenes, setScenes] = useState(() => (initialScenes?.length ? initialScenes.map((s) => ({ - key: uuidv4(), sceneId: s.sceneId || uuidv4(), location: s.location || '', - lines: s.lines.map((l) => ({ key: uuidv4(), type: l.type, speaker: l.speaker || '', voice: l.voice || '', text: l.text || '' })), - })) : [emptyScene()])); + const [scenes, setScenes] = useState(() => (initialScenes?.length ? scenesFromImport(initialScenes) : [emptyScene()])); const [styleDescriptor, setStyleDescriptor] = useState(''); const [castEntries, setCastEntries] = useState([]); const [locationEntries, setLocationEntries] = useState([]); @@ -211,6 +215,18 @@ export default function EpisodeComposer({ initialScenes, onQueued }) { const [previewLoading, setPreviewLoading] = useState(false); const [queuedJobId, setQueuedJobId] = useState(null); const debounceRef = useRef(null); + const previewRequestRef = useRef(0); + + // `initialScenes` starts null and arrives later once VideoGen's FableLoom + // import fetch resolves (EpisodeComposer is already mounted by then, so the + // lazy useState initializer above never sees it) — apply it the one time it + // transitions from empty to populated, without clobbering user edits after. + const importAppliedRef = useRef(!!initialScenes?.length); + useEffect(() => { + if (importAppliedRef.current || !initialScenes?.length) return; + importAppliedRef.current = true; + setScenes(scenesFromImport(initialScenes)); + }, [initialScenes]); const bible = useMemo(() => buildBible(styleDescriptor, castEntries, locationEntries), [styleDescriptor, castEntries, locationEntries]); const sanitizedScenes = useMemo(() => sanitizeScenesForRequest(scenes), [scenes]); @@ -226,10 +242,14 @@ export default function EpisodeComposer({ initialScenes, onQueued }) { if (queuedJobId) return undefined; if (!hasContent) { setPreview(null); setPreviewLoading(false); return undefined; } setPreviewLoading(true); + const requestId = (previewRequestRef.current += 1); debounceRef.current = setTimeout(async () => { const result = await lintContinuousVideoEpisode({ scenes: sanitizedScenes, bible, framings: framingsArrayFor(preview?.clips, framings), }).catch(() => null); + // A newer edit may have started a later request while this one was in + // flight — a slower earlier response must not overwrite it. + if (requestId !== previewRequestRef.current) return; setPreview(result); setPreviewLoading(false); }, PREVIEW_DEBOUNCE_MS); @@ -255,7 +275,12 @@ export default function EpisodeComposer({ initialScenes, onQueued }) { setQueuedJobId(result.jobId); }, { errorMessage: 'Failed to queue episode' }); - const canQueue = hasContent && preview?.lint?.pass === true && !queuing && !queuedJobId; + // `!previewLoading` matters: without it, editing a passing draft into a + // failing one leaves the OLD passing preview (and its stale `preview.clips`) + // queueable until the new lint resolves — Queue would fire against + // in-flight-stale scenes and the server would 422 on a lint the UI never + // showed as failing. + const canQueue = hasContent && !previewLoading && preview?.lint?.pass === true && !queuing && !queuedJobId; const resetForNewEpisode = () => { setQueuedJobId(null); @@ -279,8 +304,8 @@ export default function EpisodeComposer({ initialScenes, onQueued }) { />
- - + +
diff --git a/client/src/components/videoGen/EpisodeComposer.test.jsx b/client/src/components/videoGen/EpisodeComposer.test.jsx index 1c7d164964..fe682fa3f9 100644 --- a/client/src/components/videoGen/EpisodeComposer.test.jsx +++ b/client/src/components/videoGen/EpisodeComposer.test.jsx @@ -87,4 +87,15 @@ describe('EpisodeComposer', () => { act(() => lastEventSource().emit({ type: 'complete', result: { jobId: 'job-1' } })); expect(await screen.findByText(/Compose another episode/i)).toBeTruthy(); }); + + it('applies a FableLoom import that arrives after mount (async loom fetch)', async () => { + lintContinuousVideoEpisode.mockResolvedValue(PASSING_PREVIEW); + // VideoGen mounts EpisodeComposer before its getLoom() fetch resolves — + // initialScenes starts null/undefined and updates on a later render. + const { rerender } = render(); + expect(screen.queryByDisplayValue('Imported scene')).toBeNull(); + + rerender(); + expect(await screen.findByDisplayValue('Imported scene')).toBeTruthy(); + }); }); diff --git a/client/src/lib/episodeSceneImport.js b/client/src/lib/episodeSceneImport.js index ebe1392dd2..d5ea7f0bc3 100644 --- a/client/src/lib/episodeSceneImport.js +++ b/client/src/lib/episodeSceneImport.js @@ -4,7 +4,7 @@ // prose/teleplay text is free-form — this is a lossy starting draft the // Episode Composer always leaves user-editable, not a round-trip parser. -const CUE_RE = /^[A-Z][A-Z0-9 .,'()-]{1,60}$/; +const CUE_RE = /^[A-Z][A-Z0-9 .,'()-]{1,59}$/; // total length capped at 60 (matches looksLikeCue's guard) /** True for a short all-caps line that reads as a teleplay character cue. */ const looksLikeCue = (line) => { diff --git a/client/src/pages/VideoGen.jsx b/client/src/pages/VideoGen.jsx index 2dab94fd38..05e791215e 100644 --- a/client/src/pages/VideoGen.jsx +++ b/client/src/pages/VideoGen.jsx @@ -130,11 +130,16 @@ export default function VideoGen() { const episodeLoomId = searchParams.get('loomId'); const episodeEpisodeId = searchParams.get('episodeId'); const openEpisodeComposer = () => setSearchParams(prev => { const n = new URLSearchParams(prev); n.set('episode', '1'); return n; }); - const closeEpisodeComposer = () => setSearchParams(prev => { - const n = new URLSearchParams(prev); - n.delete('episode'); n.delete('loomId'); n.delete('episodeId'); - return n; - }); + const closeEpisodeComposer = () => { + setSearchParams(prev => { + const n = new URLSearchParams(prev); + n.delete('episode'); n.delete('loomId'); n.delete('episodeId'); + return n; + }); + // The composer unmounts on close (conditional render below) — clear the + // import so a later open with no loomId/episodeId doesn't inherit it. + setEpisodeImportScenes(null); + }; const [episodeImportScenes, setEpisodeImportScenes] = useState(null); useEffect(() => { if (!episodeOpen || !episodeLoomId || !episodeEpisodeId) return; From 9fe167921cd59f40a6465b2484547a90954af2fb Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Fri, 4 Sep 2026 14:55:15 +0000 Subject: [PATCH 022/194] fix(cos): ensure CoS agent panel retains w-full and min-w-0 when non-canvas avatar is active --- client/src/pages/ChiefOfStaff.jsx | 18 +++++++++++------ client/src/pages/ChiefOfStaff.test.jsx | 27 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/client/src/pages/ChiefOfStaff.jsx b/client/src/pages/ChiefOfStaff.jsx index e9b4fce27a..e211975336 100644 --- a/client/src/pages/ChiefOfStaff.jsx +++ b/client/src/pages/ChiefOfStaff.jsx @@ -922,7 +922,7 @@ export default function ChiefOfStaff() { {desktopPanelCollapsed ? (
) : ( -
+