Skip to content

Commit fd1d78b

Browse files
author
Syring, Nikolas
committed
fix(server): idempotent fileRead({ifExists: true}) for race-prone batch reads
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.
1 parent 1289793 commit fd1d78b

12 files changed

Lines changed: 257 additions & 120 deletions

File tree

app/L0/_all/mod/_core/admin/views/agent/storage.js

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,6 @@ function getRuntime() {
3737
return runtime;
3838
}
3939

40-
function isMissingFileError(error) {
41-
const message = String(error?.message || "");
42-
return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message);
43-
}
44-
4540
function isSingleUserAppRuntime(runtime) {
4641
return Boolean(runtime?.config?.get?.("SINGLE_USER_APP", false));
4742
}
@@ -198,16 +193,20 @@ async function buildStoredConfigPayload(runtime, { settings, systemPrompt }) {
198193
export async function loadAdminChatConfig() {
199194
const runtime = getRuntime();
200195

196+
// Idempotent read: a fresh user has no `~/conf/admin-chat.yaml` yet.
197+
// ifExists returns content: null instead of throwing 404.
198+
let result;
201199
try {
202-
const result = await runtime.api.fileRead(config.ADMIN_CHAT_CONFIG_PATH);
203-
return normalizeStoredConfig(runtime, runtime.utils.yaml.parse(String(result?.content || "")));
200+
result = await runtime.api.fileRead(config.ADMIN_CHAT_CONFIG_PATH, "utf8", { ifExists: true });
204201
} catch (error) {
205-
if (isMissingFileError(error)) {
206-
return createDefaultConfig();
207-
}
208-
209202
throw new Error(`Unable to load admin chat config: ${error.message}`);
210203
}
204+
205+
if (typeof result?.content !== "string") {
206+
return createDefaultConfig();
207+
}
208+
209+
return normalizeStoredConfig(runtime, runtime.utils.yaml.parse(result.content));
211210
}
212211

213212
export async function saveAdminChatConfig(nextConfig) {
@@ -229,20 +228,27 @@ export async function saveAdminChatConfig(nextConfig) {
229228
export async function loadAdminChatHistory() {
230229
const runtime = getRuntime();
231230

231+
// Idempotent read: history file may not exist on first run.
232+
let result;
232233
try {
233-
const result = await runtime.api.fileRead(config.ADMIN_CHAT_HISTORY_PATH);
234-
const parsed = JSON.parse(String(result?.content || "[]"));
235-
return Array.isArray(parsed) ? parsed : [];
234+
result = await runtime.api.fileRead(config.ADMIN_CHAT_HISTORY_PATH, "utf8", { ifExists: true });
236235
} catch (error) {
237-
if (isMissingFileError(error)) {
238-
return [];
239-
}
236+
throw new Error(`Unable to load admin chat history: ${error.message}`);
237+
}
238+
239+
if (typeof result?.content !== "string") {
240+
return [];
241+
}
240242

243+
try {
244+
const parsed = JSON.parse(result.content || "[]");
245+
return Array.isArray(parsed) ? parsed : [];
246+
} catch (error) {
241247
if (error instanceof SyntaxError) {
242248
throw new Error("Unable to load admin chat history: invalid JSON.");
243249
}
244250

245-
throw new Error(`Unable to load admin chat history: ${error.message}`);
251+
throw error;
246252
}
247253
}
248254

app/L0/_all/mod/_core/agent/storage.js

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,15 @@ function getRuntime() {
1818
return runtime;
1919
}
2020

21-
function isMissingFileError(error) {
22-
const message = String(error?.message || "");
23-
return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message) || /Path not found\./u.test(message);
24-
}
25-
2621
export async function loadAgentPersonality() {
2722
const runtime = getRuntime();
2823

24+
// Idempotent read: this config is optional. Use ifExists so a missing
25+
// file returns content: null instead of throwing 404.
2926
try {
30-
const result = await runtime.api.fileRead(AGENT_PERSONALITY_PATH);
27+
const result = await runtime.api.fileRead(AGENT_PERSONALITY_PATH, "utf8", { ifExists: true });
3128
return String(result?.content || "");
3229
} catch (error) {
33-
if (isMissingFileError(error)) {
34-
return "";
35-
}
36-
3730
throw new Error(`Unable to load agent personality: ${error.message}`);
3831
}
3932
}

app/L0/_all/mod/_core/dashboard_welcome/dashboard-prefs.js

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,6 @@ function getRuntime() {
2929
return runtime;
3030
}
3131

32-
function isMissingFileError(error) {
33-
const message = String(error?.message || "");
34-
return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message) || /Path not found\./u.test(message);
35-
}
36-
3732
function parseStoredBoolean(value) {
3833
if (value === true || value === false) {
3934
return value;
@@ -92,14 +87,13 @@ export function subscribeDashboardWelcomeHiddenChange(callback) {
9287
export async function loadDashboardPrefs() {
9388
const runtime = getRuntime();
9489

90+
// Idempotent read: a fresh user has no `~/conf/dashboard.yaml` yet. Use
91+
// ifExists so the missing file returns content: null instead of throwing
92+
// 404 (and triggering DevTools console noise on every space switch).
9593
try {
96-
const result = await runtime.api.fileRead(DASHBOARD_CONFIG_PATH);
94+
const result = await runtime.api.fileRead(DASHBOARD_CONFIG_PATH, "utf8", { ifExists: true });
9795
return normalizeDashboardPrefs(runtime.utils.yaml.parse(String(result?.content || "")));
9896
} catch (error) {
99-
if (isMissingFileError(error)) {
100-
return normalizeDashboardPrefs({});
101-
}
102-
10397
throw new Error(`Unable to load dashboard settings: ${error.message}`);
10498
}
10599
}

app/L0/_all/mod/_core/framework/js/api-client.js

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,11 +350,23 @@ function serializeStableValue(value) {
350350
.join(",")}}`;
351351
}
352352

353-
function createFileReadRequest(pathOrFiles, encoding) {
353+
function createFileReadRequest(pathOrFiles, encoding, options) {
354+
// `ifExists: true` opts into idempotent read semantics: missing paths
355+
// resolve to a 200 response with `content: null` (singular form) or
356+
// listed under `skipped` (batch form) instead of throwing 404. The
357+
// option arrives either through the third positional argument
358+
// (bare-path / array forms) or as an `ifExists` field on the input
359+
// object (object forms); the input object wins when both are set.
360+
const optionsObject = isPlainObject(options) ? options : {};
361+
const ifExistsFlag = optionsObject.ifExists === true;
362+
const ifExistsBody = ifExistsFlag ? { ifExists: true } : {};
363+
const ifExistsQuery = ifExistsFlag ? { ifExists: "1" } : {};
364+
354365
if (Array.isArray(pathOrFiles)) {
355366
return {
356367
method: "POST",
357368
body: {
369+
...ifExistsBody,
358370
encoding,
359371
files: pathOrFiles
360372
}
@@ -365,6 +377,8 @@ function createFileReadRequest(pathOrFiles, encoding) {
365377
return {
366378
method: "POST",
367379
body: {
380+
...ifExistsBody,
381+
...(typeof pathOrFiles.ifExists === "boolean" ? { ifExists: pathOrFiles.ifExists } : {}),
368382
encoding: pathOrFiles.encoding ?? encoding,
369383
files: pathOrFiles.files
370384
}
@@ -375,6 +389,8 @@ function createFileReadRequest(pathOrFiles, encoding) {
375389
return {
376390
method: "POST",
377391
body: {
392+
...ifExistsBody,
393+
...(typeof pathOrFiles.ifExists === "boolean" ? { ifExists: pathOrFiles.ifExists } : {}),
378394
encoding: pathOrFiles.encoding ?? encoding,
379395
path: pathOrFiles.path
380396
}
@@ -384,6 +400,7 @@ function createFileReadRequest(pathOrFiles, encoding) {
384400
return {
385401
method: "GET",
386402
query: {
403+
...ifExistsQuery,
387404
encoding,
388405
path: pathOrFiles
389406
}
@@ -1000,11 +1017,33 @@ export function createApiClient(options = {}) {
10001017
* `~` or `~/...` shorthand for the current user's `L2/<username>/...` path.
10011018
* It also accepts composed batch input through a `files` array.
10021019
*
1020+
* Pass `{ ifExists: true }` (either on the input object for the
1021+
* `{path}` / `{files}` forms or as a third positional argument for the
1022+
* bare-path / array forms) to opt into idempotent semantics: paths that
1023+
* do not exist return `200`. The singular form returns
1024+
* `{ content: null, encoding: null, path: null, skipped: [requested] }`;
1025+
* the batch form returns the read files plus a `skipped[]` field for
1026+
* any missing entries. Without `ifExists` the call stays strict so
1027+
* callers that need authoritative 404 keep their existing behaviour.
1028+
*
1029+
* Idempotent reads bypass the file-read batching queue so the option
1030+
* is applied per-call and missing paths cannot poison a shared batch.
1031+
*
10031032
* @param {string | FileReadInput[] | FileReadBatchOptions | FileReadInput} pathOrFiles
10041033
* @param {string} [encoding]
1034+
* @param {{ ifExists?: boolean }} [options]
10051035
* @returns {Promise<FileApiResult | FileBatchApiResult>}
10061036
*/
1007-
async function fileRead(pathOrFiles, encoding = "utf8") {
1037+
async function fileRead(pathOrFiles, encoding = "utf8", options) {
1038+
const optionsObject = isPlainObject(options) ? options : {};
1039+
const inputIfExists =
1040+
isPlainObject(pathOrFiles) && typeof pathOrFiles.ifExists === "boolean" ? pathOrFiles.ifExists : null;
1041+
const ifExistsFlag = inputIfExists === true || optionsObject.ifExists === true;
1042+
1043+
if (ifExistsFlag) {
1044+
return call("file_read", createFileReadRequest(pathOrFiles, encoding, { ifExists: true }));
1045+
}
1046+
10081047
return queueFileRead(pathOrFiles, encoding);
10091048
}
10101049

app/L0/_all/mod/_core/login_hooks/login-hooks.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,12 @@ async function hasFirstLoginMarker(runtime, markerPath) {
100100
}
101101
}
102102

103+
// fileRead-fallback when fileInfo is not available. Idempotent read so a
104+
// missing marker resolves cleanly without console-spamming a 404.
103105
try {
104-
await runtime.api.fileRead(markerPath);
105-
return true;
106+
const result = await runtime.api.fileRead(markerPath, "utf8", { ifExists: true });
107+
return typeof result?.content === "string";
106108
} catch (error) {
107-
if (isMissingFileError(error)) {
108-
return false;
109-
}
110-
111109
throw error;
112110
}
113111
}

app/L0/_all/mod/_core/onscreen_agent/storage.js

Lines changed: 58 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,6 @@ function getRuntime() {
124124
return runtime;
125125
}
126126

127-
function isMissingFileError(error) {
128-
const message = String(error?.message || "");
129-
return /\bstatus 404\b/u.test(message) || /File not found\./u.test(message);
130-
}
131-
132127
function isSingleUserAppRuntime(runtime) {
133128
return Boolean(runtime?.config?.get?.("SINGLE_USER_APP", false));
134129
}
@@ -388,52 +383,58 @@ export async function loadOnscreenAgentConfig() {
388383
const runtime = getRuntime();
389384
const uiStateOwner = await getUiStateOwner(runtime);
390385

386+
// Idempotent read: a fresh user has no `~/conf/onscreen-agent.yaml` yet.
387+
// Use ifExists so the missing file returns content: null instead of
388+
// throwing 404 on every space switch and console-spamming the user.
389+
let result;
391390
try {
392-
const result = await runtime.api.fileRead(config.ONSCREEN_AGENT_CONFIG_PATH);
393-
const normalizedConfig = await normalizeStoredConfig(
394-
runtime,
395-
runtime.utils.yaml.parse(String(result?.content || ""))
396-
);
391+
result = await runtime.api.fileRead(config.ONSCREEN_AGENT_CONFIG_PATH, "utf8", { ifExists: true });
392+
} catch (error) {
393+
throw new Error(`Unable to load onscreen agent config: ${error.message}`);
394+
}
395+
396+
if (typeof result?.content !== "string") {
397+
// Missing config: fall through to first-run defaults with optional UI state replay.
397398
const storedUiState =
398-
loadUiStateFromStorageArea("sessionStorage", { owner: uiStateOwner }) ||
399-
loadUiStateFromStorageArea("localStorage", { owner: uiStateOwner }) ||
400-
normalizeStoredUiState(normalizedConfig);
399+
loadUiStateFromStorageArea("sessionStorage", { allowUnowned: false, owner: uiStateOwner }) ||
400+
loadUiStateFromStorageArea("localStorage", { allowUnowned: false, owner: uiStateOwner });
401+
const defaultConfig = createDefaultConfig();
401402

402-
return {
403-
settings: normalizedConfig.settings,
404-
systemPrompt: normalizedConfig.systemPrompt,
405-
...storedUiState,
406-
uiStateOwner,
407-
shouldCenterInitialPosition: false
408-
};
409-
} catch (error) {
410-
if (isMissingFileError(error)) {
411-
const storedUiState =
412-
loadUiStateFromStorageArea("sessionStorage", { allowUnowned: false, owner: uiStateOwner }) ||
413-
loadUiStateFromStorageArea("localStorage", { allowUnowned: false, owner: uiStateOwner });
414-
const defaultConfig = createDefaultConfig();
415-
416-
if (storedUiState) {
417-
return {
418-
settings: defaultConfig.settings,
419-
systemPrompt: defaultConfig.systemPrompt,
420-
...storedUiState,
421-
uiStateOwner,
422-
shouldCenterInitialPosition: false
423-
};
424-
}
425-
426-
// A missing per-user config with no owner-tagged UI state means first-run defaults for this load.
403+
if (storedUiState) {
427404
return {
428-
...defaultConfig,
429-
...createDefaultUiState(),
405+
settings: defaultConfig.settings,
406+
systemPrompt: defaultConfig.systemPrompt,
407+
...storedUiState,
430408
uiStateOwner,
431-
shouldCenterInitialPosition: true
409+
shouldCenterInitialPosition: false
432410
};
433411
}
434412

435-
throw new Error(`Unable to load onscreen agent config: ${error.message}`);
413+
// A missing per-user config with no owner-tagged UI state means first-run defaults for this load.
414+
return {
415+
...defaultConfig,
416+
...createDefaultUiState(),
417+
uiStateOwner,
418+
shouldCenterInitialPosition: true
419+
};
436420
}
421+
422+
const normalizedConfig = await normalizeStoredConfig(
423+
runtime,
424+
runtime.utils.yaml.parse(result.content)
425+
);
426+
const storedUiState =
427+
loadUiStateFromStorageArea("sessionStorage", { owner: uiStateOwner }) ||
428+
loadUiStateFromStorageArea("localStorage", { owner: uiStateOwner }) ||
429+
normalizeStoredUiState(normalizedConfig);
430+
431+
return {
432+
settings: normalizedConfig.settings,
433+
systemPrompt: normalizedConfig.systemPrompt,
434+
...storedUiState,
435+
uiStateOwner,
436+
shouldCenterInitialPosition: false
437+
};
437438
}
438439

439440
export async function saveOnscreenAgentConfig(nextConfig) {
@@ -460,20 +461,28 @@ export function saveOnscreenAgentUiState(nextState) {
460461
export async function loadOnscreenAgentHistory() {
461462
const runtime = getRuntime();
462463

464+
// Idempotent read: history file may not exist on first run. ifExists
465+
// returns content: null instead of throwing 404.
466+
let result;
463467
try {
464-
const result = await runtime.api.fileRead(config.ONSCREEN_AGENT_HISTORY_PATH);
465-
const parsed = JSON.parse(String(result?.content || "[]"));
466-
return Array.isArray(parsed) ? parsed : [];
468+
result = await runtime.api.fileRead(config.ONSCREEN_AGENT_HISTORY_PATH, "utf8", { ifExists: true });
467469
} catch (error) {
468-
if (isMissingFileError(error)) {
469-
return [];
470-
}
470+
throw new Error(`Unable to load onscreen agent history: ${error.message}`);
471+
}
472+
473+
if (typeof result?.content !== "string") {
474+
return [];
475+
}
471476

477+
try {
478+
const parsed = JSON.parse(result.content || "[]");
479+
return Array.isArray(parsed) ? parsed : [];
480+
} catch (error) {
472481
if (error instanceof SyntaxError) {
473482
throw new Error("Unable to load onscreen agent history: invalid JSON.");
474483
}
475484

476-
throw new Error(`Unable to load onscreen agent history: ${error.message}`);
485+
throw error;
477486
}
478487
}
479488

app/L0/_all/mod/_core/panels/panel-index.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,14 @@ export async function listPanels() {
251251
return [];
252252
}
253253

254+
// Idempotent batch read: panel manifests can be removed by module_remove
255+
// between the listing and this read. The panels parser keys files by
256+
// path through a Map, so missing entries simply do not appear in the
257+
// lookup — same shape we get from a 200 with `skipped`, just without
258+
// the 404 console noise.
254259
const result = await runtime.api.fileRead({
255-
files: manifestFiles.map((manifestFile) => manifestFile.filePath)
260+
files: manifestFiles.map((manifestFile) => manifestFile.filePath),
261+
ifExists: true
256262
});
257263
const files = Array.isArray(result?.files) ? result.files : [];
258264
const fileMap = new Map(

0 commit comments

Comments
 (0)