From fd1d78bdbface98fe2c93acba417de7e67ec059e Mon Sep 17 00:00:00 2001 From: "Syring, Nikolas" Date: Mon, 27 Apr 2026 07:25:27 +0200 Subject: [PATCH] fix(server): idempotent fileRead({ifExists: true}) for race-prone batch reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pathologies were silently logging 404s in DevTools and routing through installFetchProxy's retry path on every space switch and config-load: 1. Batch readers in spaces/storage.js (readSpace, parseWidgetFiles batch, listSpaces) walk the path index and then read all matched files. A widget or manifest can be deleted between the index walk and the read (concurrent space deletion, file_explorer rename, watchdog catching up), failing the entire batch. 2. Optional config readers across the codebase load files like ~/conf/dashboard.yaml, ~/conf/onscreen-agent.yaml, ~/conf/personality.system.include.md, ~/user.yaml. On a fresh user account these never existed, so the load wraps the call in try/catch + isMissingFileError and treats 404 as "use defaults". The 404 is suppressed in JS but still logged by the browser before JavaScript can intercept it. This PR adds an opt-in `ifExists: true` option to fileRead, mirroring the shape of fileDelete({ifExists: true}) (PR #35). Server returns 200; missing paths appear under `skipped[]`; the singular form returns {content: null, encoding: null, path: null, skipped: [requested]}. Strict semantics remain the default for user-initiated reads and known-must-exist paths so a real "this resource is gone" diagnostic still surfaces. Server side: - normalizeReadRequests reads options.ifExists; missing paths go into skipped[] instead of throwing 404. Other resolution failures (empty path, directory-instead-of-file, public-only, permission) still throw. - readAppFiles surfaces skipped on the result envelope when non-empty and additionally catches ENOENT from fs.readFileSync when ifExists is set, closing the race where the path index says the file is there but it has been removed externally since the last scan. - readAppFile keeps its singular contract; returns content: null / encoding: null / path: null when the only path was skipped. - file_read endpoint reads ifExists from POST body or GET query (?ifExists=1) so the bare-path GET form keeps working without forcing every idempotent read into POST. Client side: - createFileReadRequest accepts a third positional argument for the bare-path/array forms and an ifExists field on the object forms. Strict bodies/queries byte-identical to today. - fileRead branches: idempotent reads bypass the file-read batching queue and call directly into call("file_read", ...). Mixing strict and idempotent modes in one batch would change the strict caller's behaviour, so the bypass keeps each idempotent call's semantic isolated. The queue's batching benefit is preserved for the strict default. Migrated callers: Pathology 1 (race-prone batch reads in spaces/storage.js): - readSpace(...) — manifest + widgets batch - parseWidgetFiles(...) batch — widget batch - listSpaces(...) bulk — manifest+widgets across all spaces Pathology 2 (optional config readers; previously try/catch + 404): - agent/storage.js loadAgentPersonality - user/storage.js readUserConfig - dashboard_welcome/dashboard-prefs.js loadDashboardPrefs - onscreen_agent/storage.js loadOnscreenAgentConfig + loadOnscreenAgentHistory - admin/views/agent/storage.js loadAdminChatConfig + loadAdminChatHistory - login_hooks/login-hooks.js hasFirstLoginMarker (fileRead fallback path) - panels/panel-index.js listPanels batch Each of these had its own copy of the isMissingFileError(...) helper for the same regex. Where the helper has no remaining users in a file, this PR removes it; module_remove and other strict callers keep their copies. server/api/AGENTS.md documents the new option alongside the existing file_write operation-modes documentation. Strict callers untouched: readWidgetFile, readManifestFile's duplicateSpace caller, all single-file reads paired with a known-existing target. --- .../mod/_core/admin/views/agent/storage.js | 42 ++++--- app/L0/_all/mod/_core/agent/storage.js | 13 +-- .../dashboard_welcome/dashboard-prefs.js | 14 +-- .../_all/mod/_core/framework/js/api-client.js | 43 ++++++- .../_all/mod/_core/login_hooks/login-hooks.js | 10 +- .../_all/mod/_core/onscreen_agent/storage.js | 107 ++++++++++-------- app/L0/_all/mod/_core/panels/panel-index.js | 8 +- app/L0/_all/mod/_core/spaces/storage.js | 27 ++++- app/L0/_all/mod/_core/user/storage.js | 13 +-- server/api/AGENTS.md | 1 + server/api/file_read.js | 23 ++++ server/lib/customware/file_access.js | 76 +++++++++++-- 12 files changed, 257 insertions(+), 120 deletions(-) diff --git a/app/L0/_all/mod/_core/admin/views/agent/storage.js b/app/L0/_all/mod/_core/admin/views/agent/storage.js index 2dbeb99d..100b0a7a 100644 --- a/app/L0/_all/mod/_core/admin/views/agent/storage.js +++ b/app/L0/_all/mod/_core/admin/views/agent/storage.js @@ -37,11 +37,6 @@ function getRuntime() { return runtime; } -function isMissingFileError(error) { - const message = String(error?.message || ""); - return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message); -} - function isSingleUserAppRuntime(runtime) { return Boolean(runtime?.config?.get?.("SINGLE_USER_APP", false)); } @@ -198,16 +193,20 @@ async function buildStoredConfigPayload(runtime, { settings, systemPrompt }) { export async function loadAdminChatConfig() { const runtime = getRuntime(); + // Idempotent read: a fresh user has no `~/conf/admin-chat.yaml` yet. + // ifExists returns content: null instead of throwing 404. + let result; try { - const result = await runtime.api.fileRead(config.ADMIN_CHAT_CONFIG_PATH); - return normalizeStoredConfig(runtime, runtime.utils.yaml.parse(String(result?.content || ""))); + result = await runtime.api.fileRead(config.ADMIN_CHAT_CONFIG_PATH, "utf8", { ifExists: true }); } catch (error) { - if (isMissingFileError(error)) { - return createDefaultConfig(); - } - throw new Error(`Unable to load admin chat config: ${error.message}`); } + + if (typeof result?.content !== "string") { + return createDefaultConfig(); + } + + return normalizeStoredConfig(runtime, runtime.utils.yaml.parse(result.content)); } export async function saveAdminChatConfig(nextConfig) { @@ -229,20 +228,27 @@ export async function saveAdminChatConfig(nextConfig) { export async function loadAdminChatHistory() { const runtime = getRuntime(); + // Idempotent read: history file may not exist on first run. + let result; try { - const result = await runtime.api.fileRead(config.ADMIN_CHAT_HISTORY_PATH); - const parsed = JSON.parse(String(result?.content || "[]")); - return Array.isArray(parsed) ? parsed : []; + result = await runtime.api.fileRead(config.ADMIN_CHAT_HISTORY_PATH, "utf8", { ifExists: true }); } catch (error) { - if (isMissingFileError(error)) { - return []; - } + throw new Error(`Unable to load admin chat history: ${error.message}`); + } + + if (typeof result?.content !== "string") { + return []; + } + try { + const parsed = JSON.parse(result.content || "[]"); + return Array.isArray(parsed) ? parsed : []; + } catch (error) { if (error instanceof SyntaxError) { throw new Error("Unable to load admin chat history: invalid JSON."); } - throw new Error(`Unable to load admin chat history: ${error.message}`); + throw error; } } diff --git a/app/L0/_all/mod/_core/agent/storage.js b/app/L0/_all/mod/_core/agent/storage.js index b15211a5..1e7ce2fe 100644 --- a/app/L0/_all/mod/_core/agent/storage.js +++ b/app/L0/_all/mod/_core/agent/storage.js @@ -18,22 +18,15 @@ function getRuntime() { return runtime; } -function isMissingFileError(error) { - const message = String(error?.message || ""); - return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message) || /Path not found\./u.test(message); -} - export async function loadAgentPersonality() { const runtime = getRuntime(); + // Idempotent read: this config is optional. Use ifExists so a missing + // file returns content: null instead of throwing 404. try { - const result = await runtime.api.fileRead(AGENT_PERSONALITY_PATH); + const result = await runtime.api.fileRead(AGENT_PERSONALITY_PATH, "utf8", { ifExists: true }); return String(result?.content || ""); } catch (error) { - if (isMissingFileError(error)) { - return ""; - } - throw new Error(`Unable to load agent personality: ${error.message}`); } } diff --git a/app/L0/_all/mod/_core/dashboard_welcome/dashboard-prefs.js b/app/L0/_all/mod/_core/dashboard_welcome/dashboard-prefs.js index 05978292..4843007d 100644 --- a/app/L0/_all/mod/_core/dashboard_welcome/dashboard-prefs.js +++ b/app/L0/_all/mod/_core/dashboard_welcome/dashboard-prefs.js @@ -29,11 +29,6 @@ function getRuntime() { return runtime; } -function isMissingFileError(error) { - const message = String(error?.message || ""); - return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message) || /Path not found\./u.test(message); -} - function parseStoredBoolean(value) { if (value === true || value === false) { return value; @@ -92,14 +87,13 @@ export function subscribeDashboardWelcomeHiddenChange(callback) { export async function loadDashboardPrefs() { const runtime = getRuntime(); + // Idempotent read: a fresh user has no `~/conf/dashboard.yaml` yet. Use + // ifExists so the missing file returns content: null instead of throwing + // 404 (and triggering DevTools console noise on every space switch). try { - const result = await runtime.api.fileRead(DASHBOARD_CONFIG_PATH); + const result = await runtime.api.fileRead(DASHBOARD_CONFIG_PATH, "utf8", { ifExists: true }); return normalizeDashboardPrefs(runtime.utils.yaml.parse(String(result?.content || ""))); } catch (error) { - if (isMissingFileError(error)) { - return normalizeDashboardPrefs({}); - } - throw new Error(`Unable to load dashboard settings: ${error.message}`); } } diff --git a/app/L0/_all/mod/_core/framework/js/api-client.js b/app/L0/_all/mod/_core/framework/js/api-client.js index f3d4a066..6c5f02fe 100644 --- a/app/L0/_all/mod/_core/framework/js/api-client.js +++ b/app/L0/_all/mod/_core/framework/js/api-client.js @@ -350,11 +350,23 @@ function serializeStableValue(value) { .join(",")}}`; } -function createFileReadRequest(pathOrFiles, encoding) { +function createFileReadRequest(pathOrFiles, encoding, options) { + // `ifExists: true` opts into idempotent read semantics: missing paths + // resolve to a 200 response with `content: null` (singular form) or + // listed under `skipped` (batch form) instead of throwing 404. The + // option arrives either through the third positional argument + // (bare-path / array forms) or as an `ifExists` field on the input + // object (object forms); the input object wins when both are set. + const optionsObject = isPlainObject(options) ? options : {}; + const ifExistsFlag = optionsObject.ifExists === true; + const ifExistsBody = ifExistsFlag ? { ifExists: true } : {}; + const ifExistsQuery = ifExistsFlag ? { ifExists: "1" } : {}; + if (Array.isArray(pathOrFiles)) { return { method: "POST", body: { + ...ifExistsBody, encoding, files: pathOrFiles } @@ -365,6 +377,8 @@ function createFileReadRequest(pathOrFiles, encoding) { return { method: "POST", body: { + ...ifExistsBody, + ...(typeof pathOrFiles.ifExists === "boolean" ? { ifExists: pathOrFiles.ifExists } : {}), encoding: pathOrFiles.encoding ?? encoding, files: pathOrFiles.files } @@ -375,6 +389,8 @@ function createFileReadRequest(pathOrFiles, encoding) { return { method: "POST", body: { + ...ifExistsBody, + ...(typeof pathOrFiles.ifExists === "boolean" ? { ifExists: pathOrFiles.ifExists } : {}), encoding: pathOrFiles.encoding ?? encoding, path: pathOrFiles.path } @@ -384,6 +400,7 @@ function createFileReadRequest(pathOrFiles, encoding) { return { method: "GET", query: { + ...ifExistsQuery, encoding, path: pathOrFiles } @@ -1000,11 +1017,33 @@ export function createApiClient(options = {}) { * `~` or `~/...` shorthand for the current user's `L2//...` path. * It also accepts composed batch input through a `files` array. * + * Pass `{ ifExists: true }` (either on the input object for the + * `{path}` / `{files}` forms or as a third positional argument for the + * bare-path / array forms) to opt into idempotent semantics: paths that + * do not exist return `200`. The singular form returns + * `{ content: null, encoding: null, path: null, skipped: [requested] }`; + * the batch form returns the read files plus a `skipped[]` field for + * any missing entries. Without `ifExists` the call stays strict so + * callers that need authoritative 404 keep their existing behaviour. + * + * Idempotent reads bypass the file-read batching queue so the option + * is applied per-call and missing paths cannot poison a shared batch. + * * @param {string | FileReadInput[] | FileReadBatchOptions | FileReadInput} pathOrFiles * @param {string} [encoding] + * @param {{ ifExists?: boolean }} [options] * @returns {Promise} */ - async function fileRead(pathOrFiles, encoding = "utf8") { + async function fileRead(pathOrFiles, encoding = "utf8", options) { + const optionsObject = isPlainObject(options) ? options : {}; + const inputIfExists = + isPlainObject(pathOrFiles) && typeof pathOrFiles.ifExists === "boolean" ? pathOrFiles.ifExists : null; + const ifExistsFlag = inputIfExists === true || optionsObject.ifExists === true; + + if (ifExistsFlag) { + return call("file_read", createFileReadRequest(pathOrFiles, encoding, { ifExists: true })); + } + return queueFileRead(pathOrFiles, encoding); } diff --git a/app/L0/_all/mod/_core/login_hooks/login-hooks.js b/app/L0/_all/mod/_core/login_hooks/login-hooks.js index 96ea7573..90058bb2 100644 --- a/app/L0/_all/mod/_core/login_hooks/login-hooks.js +++ b/app/L0/_all/mod/_core/login_hooks/login-hooks.js @@ -100,14 +100,12 @@ async function hasFirstLoginMarker(runtime, markerPath) { } } + // fileRead-fallback when fileInfo is not available. Idempotent read so a + // missing marker resolves cleanly without console-spamming a 404. try { - await runtime.api.fileRead(markerPath); - return true; + const result = await runtime.api.fileRead(markerPath, "utf8", { ifExists: true }); + return typeof result?.content === "string"; } catch (error) { - if (isMissingFileError(error)) { - return false; - } - throw error; } } diff --git a/app/L0/_all/mod/_core/onscreen_agent/storage.js b/app/L0/_all/mod/_core/onscreen_agent/storage.js index 6a9b22f2..6722df48 100644 --- a/app/L0/_all/mod/_core/onscreen_agent/storage.js +++ b/app/L0/_all/mod/_core/onscreen_agent/storage.js @@ -124,11 +124,6 @@ function getRuntime() { return runtime; } -function isMissingFileError(error) { - const message = String(error?.message || ""); - return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message); -} - function isSingleUserAppRuntime(runtime) { return Boolean(runtime?.config?.get?.("SINGLE_USER_APP", false)); } @@ -388,52 +383,58 @@ export async function loadOnscreenAgentConfig() { const runtime = getRuntime(); const uiStateOwner = await getUiStateOwner(runtime); + // Idempotent read: a fresh user has no `~/conf/onscreen-agent.yaml` yet. + // Use ifExists so the missing file returns content: null instead of + // throwing 404 on every space switch and console-spamming the user. + let result; try { - const result = await runtime.api.fileRead(config.ONSCREEN_AGENT_CONFIG_PATH); - const normalizedConfig = await normalizeStoredConfig( - runtime, - runtime.utils.yaml.parse(String(result?.content || "")) - ); + result = await runtime.api.fileRead(config.ONSCREEN_AGENT_CONFIG_PATH, "utf8", { ifExists: true }); + } catch (error) { + throw new Error(`Unable to load onscreen agent config: ${error.message}`); + } + + if (typeof result?.content !== "string") { + // Missing config: fall through to first-run defaults with optional UI state replay. const storedUiState = - loadUiStateFromStorageArea("sessionStorage", { owner: uiStateOwner }) || - loadUiStateFromStorageArea("localStorage", { owner: uiStateOwner }) || - normalizeStoredUiState(normalizedConfig); + loadUiStateFromStorageArea("sessionStorage", { allowUnowned: false, owner: uiStateOwner }) || + loadUiStateFromStorageArea("localStorage", { allowUnowned: false, owner: uiStateOwner }); + const defaultConfig = createDefaultConfig(); - return { - settings: normalizedConfig.settings, - systemPrompt: normalizedConfig.systemPrompt, - ...storedUiState, - uiStateOwner, - shouldCenterInitialPosition: false - }; - } catch (error) { - if (isMissingFileError(error)) { - const storedUiState = - loadUiStateFromStorageArea("sessionStorage", { allowUnowned: false, owner: uiStateOwner }) || - loadUiStateFromStorageArea("localStorage", { allowUnowned: false, owner: uiStateOwner }); - const defaultConfig = createDefaultConfig(); - - if (storedUiState) { - return { - settings: defaultConfig.settings, - systemPrompt: defaultConfig.systemPrompt, - ...storedUiState, - uiStateOwner, - shouldCenterInitialPosition: false - }; - } - - // A missing per-user config with no owner-tagged UI state means first-run defaults for this load. + if (storedUiState) { return { - ...defaultConfig, - ...createDefaultUiState(), + settings: defaultConfig.settings, + systemPrompt: defaultConfig.systemPrompt, + ...storedUiState, uiStateOwner, - shouldCenterInitialPosition: true + shouldCenterInitialPosition: false }; } - throw new Error(`Unable to load onscreen agent config: ${error.message}`); + // A missing per-user config with no owner-tagged UI state means first-run defaults for this load. + return { + ...defaultConfig, + ...createDefaultUiState(), + uiStateOwner, + shouldCenterInitialPosition: true + }; } + + const normalizedConfig = await normalizeStoredConfig( + runtime, + runtime.utils.yaml.parse(result.content) + ); + const storedUiState = + loadUiStateFromStorageArea("sessionStorage", { owner: uiStateOwner }) || + loadUiStateFromStorageArea("localStorage", { owner: uiStateOwner }) || + normalizeStoredUiState(normalizedConfig); + + return { + settings: normalizedConfig.settings, + systemPrompt: normalizedConfig.systemPrompt, + ...storedUiState, + uiStateOwner, + shouldCenterInitialPosition: false + }; } export async function saveOnscreenAgentConfig(nextConfig) { @@ -460,20 +461,28 @@ export function saveOnscreenAgentUiState(nextState) { export async function loadOnscreenAgentHistory() { const runtime = getRuntime(); + // Idempotent read: history file may not exist on first run. ifExists + // returns content: null instead of throwing 404. + let result; try { - const result = await runtime.api.fileRead(config.ONSCREEN_AGENT_HISTORY_PATH); - const parsed = JSON.parse(String(result?.content || "[]")); - return Array.isArray(parsed) ? parsed : []; + result = await runtime.api.fileRead(config.ONSCREEN_AGENT_HISTORY_PATH, "utf8", { ifExists: true }); } catch (error) { - if (isMissingFileError(error)) { - return []; - } + throw new Error(`Unable to load onscreen agent history: ${error.message}`); + } + + if (typeof result?.content !== "string") { + return []; + } + try { + const parsed = JSON.parse(result.content || "[]"); + return Array.isArray(parsed) ? parsed : []; + } catch (error) { if (error instanceof SyntaxError) { throw new Error("Unable to load onscreen agent history: invalid JSON."); } - throw new Error(`Unable to load onscreen agent history: ${error.message}`); + throw error; } } diff --git a/app/L0/_all/mod/_core/panels/panel-index.js b/app/L0/_all/mod/_core/panels/panel-index.js index 7e841bb7..47dbc106 100644 --- a/app/L0/_all/mod/_core/panels/panel-index.js +++ b/app/L0/_all/mod/_core/panels/panel-index.js @@ -251,8 +251,14 @@ export async function listPanels() { return []; } + // Idempotent batch read: panel manifests can be removed by module_remove + // between the listing and this read. The panels parser keys files by + // path through a Map, so missing entries simply do not appear in the + // lookup — same shape we get from a 200 with `skipped`, just without + // the 404 console noise. const result = await runtime.api.fileRead({ - files: manifestFiles.map((manifestFile) => manifestFile.filePath) + files: manifestFiles.map((manifestFile) => manifestFile.filePath), + ifExists: true }); const files = Array.isArray(result?.files) ? result.files : []; const fileMap = new Map( diff --git a/app/L0/_all/mod/_core/spaces/storage.js b/app/L0/_all/mod/_core/spaces/storage.js index 853f8b45..109f2621 100644 --- a/app/L0/_all/mod/_core/spaces/storage.js +++ b/app/L0/_all/mod/_core/spaces/storage.js @@ -1393,8 +1393,13 @@ async function readWidgetFiles(spaceId, widgetPaths = null) { return {}; } + // Idempotent batch read: same race window as `readSpace` — widget files can + // be deleted between the path-index listing and the read. The parser + // already accepts a partial files array, so let the server return 200 + // with any missing entries under `skipped` instead of throwing 404. const readResult = await runtime.api.fileRead({ - files: yamlWidgetPaths + files: yamlWidgetPaths, + ifExists: true }); return parseWidgetFiles(spaceId, Array.isArray(readResult?.files) ? readResult.files : []); @@ -1630,8 +1635,15 @@ export async function listSpaces() { widgetCounts[widgetSpaceId].add(widgetId); }); + // Idempotent bulk read: when listing all spaces, individual manifest or + // widget files can disappear between the path-index walk and the read + // (concurrent space deletion, file_explorer rename, watchdog catching up). + // The downstream code keys files by path through a Map, so missing entries + // simply do not appear in the lookup — the same shape we get from a 200 + // with a `skipped` field, just without the 404 noise. const readResult = await runtime.api.fileRead({ - files: [...manifestPaths, ...yamlWidgetPaths] + files: [...manifestPaths, ...yamlWidgetPaths], + ifExists: true }); const files = Array.isArray(readResult?.files) ? readResult.files : []; const fileMap = new Map( @@ -1771,8 +1783,17 @@ export async function readSpace(spaceId) { } const yamlWidgetPaths = widgetPaths.filter((path) => String(path || "").endsWith(SPACE_WIDGET_FILE_EXTENSION)); + // Idempotent batch read: a widget file or even the manifest can disappear + // between the path-index lookup and the read (space deletion races, watchdog + // catching up, external file removal). The downstream code already treats a + // missing manifest as `null`, so let the server return 200 with the missing + // path under `skipped` instead of throwing 404 and triggering the + // fetch-proxy retry path. The widget files come from `listSpaceWidgetPaths` + // which itself filters by index presence, so only race-window misses end up + // skipped here. const readResult = await runtime.api.fileRead({ - files: [buildSpaceManifestPath(normalizedSpaceId), ...yamlWidgetPaths] + files: [buildSpaceManifestPath(normalizedSpaceId), ...yamlWidgetPaths], + ifExists: true }); const files = Array.isArray(readResult?.files) ? readResult.files : []; const manifestFile = diff --git a/app/L0/_all/mod/_core/user/storage.js b/app/L0/_all/mod/_core/user/storage.js index f95b31f9..8c4eb2a4 100644 --- a/app/L0/_all/mod/_core/user/storage.js +++ b/app/L0/_all/mod/_core/user/storage.js @@ -33,11 +33,6 @@ function getRuntime() { return runtime; } -function isMissingFileError(error) { - const message = String(error?.message || ""); - return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message) || /Path not found\./u.test(message); -} - function normalizeList(values) { return Array.isArray(values) ? values.map((value) => String(value || "")).filter(Boolean) : []; } @@ -60,15 +55,13 @@ function normalizeFullName(fullName, username) { } async function readUserConfig(runtime) { + // Idempotent read: a fresh user has no `~/user.yaml` yet. Use ifExists so + // the missing file returns content: null instead of throwing 404. try { - const result = await runtime.api.fileRead(USER_CONFIG_PATH); + const result = await runtime.api.fileRead(USER_CONFIG_PATH, "utf8", { ifExists: true }); const parsed = runtime.utils.yaml.parse(String(result?.content || "")); return parsed && typeof parsed === "object" && !Array.isArray(parsed) ? parsed : {}; } catch (error) { - if (isMissingFileError(error)) { - return {}; - } - throw new Error(`Unable to load ${USER_CONFIG_PATH}: ${error.message}`); } } diff --git a/server/api/AGENTS.md b/server/api/AGENTS.md index 0475d294..9048afc1 100644 --- a/server/api/AGENTS.md +++ b/server/api/AGENTS.md @@ -68,6 +68,7 @@ Current rules: - `file_paths` also accepts an optional explicit `maxLayer` body or query value when module-oriented discovery should ignore higher writable layers; this is used by the admin agent skill catalog so firmware-backed `ext/skills/` files are not shadowed by L1 or L2 customware - batch operations validate all targets before any mutation begins - `file_write` still defaults to full-file replacement, but object-form writes also support `operation: "append"`, `"prepend"`, or `"insert"`; insert writes accept exactly one of `line`, `before`, or `after`, use the first literal `before` or `after` match, and require `utf8` encoding +- `file_read` defaults to strict semantics (404 when the path is gone). Cache-aware callers — most importantly the space and bulk-space readers, where pathIndex/disk races are inherent — can pass `ifExists: true` (POST body) or `?ifExists=1` (GET query) to opt into idempotent read: missing paths return 200 with the path under `skipped`, the singular form returns `content: null`/`encoding: null`/`path: null`, and an `ENOENT` from `fs.readFileSync` after a stale-index resolution is treated the same way - when `USER_FOLDER_SIZE_LIMIT_BYTES` is positive, `file_write`, `file_copy`, `file_move`, `file_delete`, and module removal through `file_access.js` enforce the per-`L2//` folder quota before mutation; the shared quota helper should derive current totals and subtree deltas from indexed `sizeBytes` metadata instead of rescanning the whole user tree, and quota errors return `413` - single-file or single-folder copy and move requests must keep working when request plumbing omits `entries`; only real batch calls should forward an `entries` array to the shared helper - endpoint-specific validation should stay thin and reuse the shared helper contract diff --git a/server/api/file_read.js b/server/api/file_read.js index 393eb7e0..150be67d 100644 --- a/server/api/file_read.js +++ b/server/api/file_read.js @@ -21,6 +21,22 @@ function hasBatchRead(payload) { return Boolean(payload) && typeof payload === "object" && Array.isArray(payload.files); } +function readIfExistsFlag(context) { + const payload = readPayload(context); + if (payload.ifExists === true) { + return true; + } + // GET requests carry the option as a query parameter so the helper + // signature `fileRead(path, encoding, { ifExists: true })` keeps its + // single-path GET form without forcing every idempotent read into POST. + const queryValue = context.params?.ifExists ?? context.params?.if_exists; + if (queryValue === undefined || queryValue === null) { + return false; + } + const normalizedQueryValue = String(queryValue).trim().toLowerCase(); + return normalizedQueryValue === "1" || normalizedQueryValue === "true"; +} + function handleRead(context) { const payload = readPayload(context); const maxLayer = resolveRequestMaxLayer({ @@ -28,10 +44,17 @@ function handleRead(context) { headers: context.headers, requestUrl: context.requestUrl }); + // `ifExists: true` opts into idempotent read semantics: missing paths + // resolve to a 200 response with the path listed under `skipped` + // (singular form returns `content: null`) instead of throwing 404. + // Strict callers (default) keep their authoritative 404 so user-facing + // reads can still surface a real "this resource is gone" diagnostic. + const ifExists = readIfExistsFlag(context); try { const options = { encoding: readEncoding(context), + ifExists, maxLayer, path: readPath(context), projectRoot: context.projectRoot, diff --git a/server/lib/customware/file_access.js b/server/lib/customware/file_access.js index 42ebbe13..a14f2a16 100644 --- a/server/lib/customware/file_access.js +++ b/server/lib/customware/file_access.js @@ -558,9 +558,15 @@ function normalizeReadRequests(options = {}) { runtimeParams: options.runtimeParams, username: options.username }); + // Per RFC-style idempotent read semantics: callers that can accept a + // missing file (e.g. preference loaders, manifest readers during space + // teardown) opt in with `ifExists: true`. Strict callers (default) keep + // the authoritative 404 so user-facing reads can still surface a real + // "this resource is gone" diagnostic. + const ifExists = options.ifExists === true; const entries = normalizeReadEntries(options); - - return entries.map((entry) => { + const skipped = []; + const requests = entries.flatMap((entry) => { const request = isPlainObject(entry) ? entry : { path: entry }; const requestedPath = String(request.path || "").trim(); @@ -574,6 +580,11 @@ function normalizeReadRequests(options = {}) { ); if (!resolvedPath.projectPath || !resolvedPath.exists) { + if (ifExists) { + skipped.push(requestedPath); + return []; + } + throw createHttpError(`File not found: ${requestedPath}`, 404); } @@ -584,7 +595,7 @@ function normalizeReadRequests(options = {}) { ensurePublicAppProjectPath(resolvedPath.projectPath); ensureReadableProjectPath(resolvedPath.projectPath, accessController); - return { + return [{ absolutePath: createAbsolutePath( String(options.projectRoot || ""), resolvedPath.projectPath, @@ -592,30 +603,73 @@ function normalizeReadRequests(options = {}) { ), encoding: ensureValidReadEncoding(String(request.encoding || options.encoding || "utf8").toLowerCase()), path: toAppRelativePath(resolvedPath.projectPath) - }; + }]; }); + + return { requests, skipped }; } function readAppFiles(options = {}) { - const requests = normalizeReadRequests(options); - const files = requests.map((request) => { - const buffer = fs.readFileSync(request.absolutePath); + const { requests, skipped } = normalizeReadRequests(options); + const ifExists = options.ifExists === true; + // With `ifExists: true`, also close the second race window symmetrically: + // when the path index says the file is there but it has been removed + // externally since the last scan, `fs.readFileSync` would throw `ENOENT` + // and surface as a 500. Treat that as "the file is gone" and append it + // to `skipped`, matching the pathIndex-says-missing branch in + // normalizeReadRequests. Strict reads still throw so the authoritative + // "this resource is gone" diagnostic remains. + const files = []; + const lateSkipped = []; + + requests.forEach((request) => { + let buffer; + + try { + buffer = fs.readFileSync(request.absolutePath); + } catch (error) { + if (ifExists && error?.code === "ENOENT") { + lateSkipped.push(request.path); + return; + } - return { + throw error; + } + + files.push({ content: request.encoding === "base64" ? buffer.toString("base64") : buffer.toString("utf8"), encoding: request.encoding, path: request.path - }; + }); }); - return { + const result = { count: files.length, files }; + + if (skipped.length > 0 || lateSkipped.length > 0) { + result.skipped = [...skipped, ...lateSkipped]; + } + + return result; } function readAppFile(options = {}) { - return readAppFiles(options).files[0]; + const result = readAppFiles(options); + // Match the singular-form contract: the legacy result was the single + // file object. With ifExists, a missing path resolves to skipped so + // `files[0]` is undefined; surface that case explicitly. + if (result.files.length === 0 && Array.isArray(result.skipped) && result.skipped.length > 0) { + return { + content: null, + encoding: null, + path: null, + skipped: result.skipped + }; + } + + return result.files[0]; } function resolveReadableExistingAppPath(options = {}) {