fix(server): idempotent fileRead({ifExists: true}) for race-prone batch reads

[nsyring]

fix(server): idempotent fileRead({ ifExists: true }) for race-prone batch reads

Summary

fileRead is currently always strict: reading a path that no longer exists throws HTTP 404. Several callers in the codebase already work around this with patterns like result.files.find(...) || files[0] || null because their use case (load all manifests in a space-list, batch-read all widgets while another tab might be deleting one) is naturally idempotent. Today these callers can either pre-check existence (extra round trip) or accept that the browser logs POST /api/file_read 404 whenever the pathIndex disagrees with disk.

This PR adds an opt-in ifExists: true option to fileRead, mirroring the symmetric option just landed on fileDelete. The 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.

After:

// strict (default) — throws if the path is gone:
const result = await space.api.fileRead("~/note.txt");

// idempotent — 200 with `content: null` if the path is gone:
const result = await space.api.fileRead("~/cache.tmp", "utf8", { ifExists: true });

// batch idempotent — 200 with `skipped: ["..."]` for any missing entries:
const result = await space.api.fileRead({
  files: [...manifestPaths, ...widgetPaths],
  ifExists: true
});

Why

Concrete reproduction: leaving a space, while a sibling tab or the file_explorer has just deleted a widget, generates a red POST http://127.0.0.1:NNNNN/api/file_read 404 (Not Found) in the DevTools console. The trigger is the batch fileRead({ files: [manifestPath, ...widgetPaths] }) in readSpace(...) and listSpaces(...). The path list comes from listSpaceWidgetPaths(...), which itself walks the path index — so a file that has been removed between the index walk and the read fails the entire batch. The downstream code at readSpace:1778 already accepts a partial files[] array (files.find(...) || files[0] || null), so the strict 404 is over-conservative for this use case. Without this PR, the call also routes through installFetchProxy(...)'s retry path, which on a slow filesystem produces a brief UI hitch when the user changes spaces rapidly.

The same race window exists for any pathIndex-walk-then-read pattern — and there are several in spaces/storage.js. This PR migrates the three race-prone batch readers (readSpace, parseWidgetFiles-batch read, listSpaces bulk read) to ifExists: true. User-initiated single-file reads (readWidgetFile, readManifestFile's sole caller in duplicateSpace) stay strict so a real "this resource is gone" diagnostic still surfaces if the user is acting on a fresh-cached but actually-deleted resource.

The architectural shape of this PR is symmetric to the recently-shipped fileDelete({ ifExists: true }) PR (#35): same option name, same skipped semantics on the result envelope, same race-defense pattern (catch ENOENT from fs.readFileSync and add to skipped instead of letting it surface as a 500). RFC-style idempotent-read is a recognized HTTP convention and the If-Match/If-None-Match headers are precedent for caller-controlled conditional reads.

What changed

server/lib/customware/file_access.js (+59 / -17)

normalizeReadRequests(options) now reads options.ifExists. When the flag is set, paths that resolve to nothing on disk are recorded in a skipped[] array and not added to the requests[] list. The function's return shape becomes { requests, skipped } (was: requests[]). All other resolution-failure cases (empty path, directory-instead-of-file, public-only path, read-permission) still throw, regardless of ifExists.

readAppFiles(...) adapts to the new return shape, surfaces skipped on its own 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. When ifExists skips the only path, the singular form returns { content: null, encoding: null, path: null, skipped: [requested] } so callers can branch on result.path === null.

server/api/file_read.js (+23 / -0)

The endpoint now reads the option from either the POST body (payload.ifExists === true) or a GET query parameter (?ifExists=1 / ?ifExists=true / ?if_exists=1). The query-string form keeps the single-path GET shape working — a bare-path read with ifExists does not have to switch to POST.

app/L0/_all/mod/_core/framework/js/api-client.js (+38 / -5)

createFileReadRequest(pathOrFiles, encoding, options) accepts a third positional argument for the bare-path / array forms (fileRead("~/x", "utf8", { ifExists: true })) and an ifExists field on the input object for the {path} / {files} forms (fileRead({ path: "~/x", ifExists: true })). Strict bodies are byte-identical to today.

fileRead(...) now branches: idempotent reads bypass the file-read batching queue and call directly into call("file_read", ...). The queue is designed to merge concurrent strict reads into a single batch, but mixing strict and idempotent modes in one batch would change the strict caller's behaviour — bypassing the queue keeps each idempotent call's semantic isolated. The bypass is rare (only callers that explicitly opt in) so the queue's batching benefit is preserved for the strict default.

JSDoc updated to document the new option, the singular and batch result shapes, and the queue-bypass behaviour.

app/L0/_all/mod/_core/spaces/storage.js (+22 / -5)

Three batch readers in spaces/storage.js now pass ifExists: true because the downstream code already tolerates a partial files[] array:

• readSpace(...) — manifest + widgets batch read; files.find(...) || files[0] || null handles missing entries
• parseWidgetFiles-batch caller — keys files by path through a Map; missing entries simply do not appear in the lookup
• listSpaces(...) bulk read — same Map-keyed shape

Strict callers untouched: readWidgetFile(...), readManifestFile(...)'s duplicateSpace caller, all single-file reads that are paired with a known-existing target.

server/api/AGENTS.md (+1 / -0)

Documents the new option for file_read alongside the existing file_write operation-modes documentation. Sits next to the file_delete ifExists doc that PR #35 introduces; the two operations now share an opt-in idempotency pattern.

Backward compatibility

• Without ifExists, every endpoint and helper behaviour is byte-identical to today. All existing callers (single-path reads, batch reads where the caller actually does need 404 to mean "missing") are unchanged.
• The readAppFiles(...) helper's return shape now includes an optional skipped[] field, but only when ifExists: true and at least one path was missing. Strict callers always get { count, files } exactly as before.
• The readAppFile(...) singular-form helper returns { content: null, ... } instead of undefined when an ifExists call's only path was missing. This is unreachable from any current strict caller and well-defined for new idempotent callers.
• The file-read batching queue is unchanged for strict callers; only opt-in idempotent reads bypass it.

Test plan

• node --check on every modified file passes
• Request-body and query-string shapes verified for the four call signatures × two strict/idempotent modes (eight cases): bare path GET, array POST, {path} POST, {files} POST — strict bodies/queries are byte-identical to today; idempotent variants add ifExists: true (POST body) or ifExists: "1" (GET query) next to the path/files field
• Manual verification in npm run desktop:pack build:
  • Leaving a space while a widget file is mid-deletion no longer logs POST /api/file_read 404 (Not Found) in DevTools console; the manifest+widgets batch returns 200 with skipped: [...] instead
  • Bulk space list during concurrent space removal stays clean
  • fileRead("~/missing.txt") without ifExists still throws 404 (strict semantics preserved)

Relationship to PR #35

This PR pairs with fix/file-delete-if-exists-semantics (#35), which introduced the same option name and result shape on fileDelete. The two PRs can be merged in either order. They share the design: caller-controlled opt-in to RFC-style idempotency, body-or-query-string transport, skipped[] on the result envelope, and a server-side defence against pathIndex-vs-disk races (force: true on rmSync for delete, ENOENT catch on readFileSync for read).

If both PRs land, server/api/AGENTS.md will end up with the two ifExists doc lines next to each other; merge resolution is trivial.

Out of scope (possible follow-ups)

• Symmetric idempotency on file_copy and file_move. Those have stricter semantics around source/target existence (a copy of a missing source has no defined output), so they need a separate design pass — not part of this PR.
• file_list and file_paths already tolerate missing folders gracefully today, so they do not need an ifExists option.
• Telemetry on skipped paths. The server returns skipped so callers can branch on it, but no migrated caller currently inspects the field beyond "at least one entry was missing". A future PR could log or count skipped reads to help diagnose stale-cache patterns.
• isNotFoundError(...) cleanup. spaces/storage.js still exports/uses this helper for callers that have not been migrated; a follow-up could either remove it once all idempotent-read callers have migrated or generalize it for non-read operations that genuinely need the distinction.

---

🤖 Generated with Claude Code