Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions app/L0/_all/mod/_core/framework/js/api-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,11 +440,23 @@ function createFileWriteRequest(pathOrFiles, content, encoding) {
};
}

function createFileDeleteRequest(pathOrPaths) {
function createFileDeleteRequest(pathOrPaths, options) {
const optionsObject = isPlainObject(options) ? options : {};
// `ifExists: true` requests idempotent delete semantics: paths that no
// longer exist on disk return `200` with the path listed under `skipped`
// instead of `404`. Useful for cache-cleanup callers (thumbnail pruning,
// stale layout files) that previously had to wrap each call in
// try/catch + isNotFoundError. Per RFC 7231 DELETE is idempotent, but we
// only opt into idempotency at the call site so strict callers
// (user-initiated file-explorer delete) keep their authoritative 404.
const ifExistsFlag = optionsObject.ifExists === true;
const ifExistsBody = ifExistsFlag ? { ifExists: true } : {};

if (Array.isArray(pathOrPaths)) {
return {
method: "POST",
body: {
...ifExistsBody,
paths: pathOrPaths
}
};
Expand All @@ -454,6 +466,8 @@ function createFileDeleteRequest(pathOrPaths) {
return {
method: "POST",
body: {
...ifExistsBody,
...(typeof pathOrPaths.ifExists === "boolean" ? { ifExists: pathOrPaths.ifExists } : {}),
paths: pathOrPaths.paths
}
};
Expand All @@ -463,6 +477,8 @@ function createFileDeleteRequest(pathOrPaths) {
return {
method: "POST",
body: {
...ifExistsBody,
...(typeof pathOrPaths.ifExists === "boolean" ? { ifExists: pathOrPaths.ifExists } : {}),
path: pathOrPaths.path
}
};
Expand All @@ -471,6 +487,7 @@ function createFileDeleteRequest(pathOrPaths) {
return {
method: "POST",
body: {
...ifExistsBody,
path: pathOrPaths
}
};
Expand Down Expand Up @@ -1033,11 +1050,20 @@ export function createApiClient(options = {}) {
* `L2/alice/old-folder/`, and the `~` or `~/...` shorthand for the current
* user's `L2/<username>/...` path. Directory deletes are recursive.
*
* Pass `{ ifExists: true }` (either on the input object for the
* `{path}` / `{paths}` forms or as a second positional argument for
* the bare-path / array forms) to opt into idempotent semantics: paths
* that do not exist return `200` and appear under `skipped` on the
* result instead of throwing `404`. Without `ifExists` the call stays
* strict so callers that need authoritative 404 (file-explorer UI,
* single-source-of-truth cleanups) keep their existing behaviour.
*
* @param {string | FileDeleteInput[] | FileDeleteBatchOptions | FileDeleteInput} pathOrPaths
* @param {{ ifExists?: boolean }} [options]
* @returns {Promise<FileApiResult | PathBatchApiResult>}
*/
async function fileDelete(pathOrPaths) {
return call("file_delete", createFileDeleteRequest(pathOrPaths));
async function fileDelete(pathOrPaths, options) {
return call("file_delete", createFileDeleteRequest(pathOrPaths, options));
}

/**
Expand Down
18 changes: 6 additions & 12 deletions app/L0/_all/mod/_core/spaces/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -1010,18 +1010,12 @@ function createHtmlRendererSource(htmlSource) {

async function deleteAppPathIfExists(path) {
const runtime = ensureSpaceRuntime();

try {
await runtime.api.fileDelete(path);
} catch (error) {
if (isNotFoundError(error)) {
return false;
}

throw error;
}

return true;
// Idempotent delete: server returns 200 with the path under `skipped`
// when the file is already gone. The `skipped` field is what tells us
// whether anything was actually removed, so callers that branch on
// "did we delete something" still get a meaningful boolean.
const result = await runtime.api.fileDelete(path, { ifExists: true });
return Array.isArray(result?.skipped) ? result.skipped.length === 0 : true;
}

function normalizeLegacyFunctionSource(sourceText) {
Expand Down
24 changes: 9 additions & 15 deletions app/L0/_all/mod/_core/spaces/thumbnail_experiment/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ function normalizeSpaceThumbnailId(value) {
return String(value || "").trim().replace(/^\/+|\/+$/gu, "");
}

function isNotFoundError(error) {
const message = String(error?.message || "").toLowerCase();
return message.includes("status 404") || message.includes("file not found") || message.includes("path not found");
}

function ensureThumbnailRuntime() {
if (!globalThis.space?.api?.fileDelete || !globalThis.space?.api?.fileWrite) {
throw new Error("Space thumbnail capture requires the authenticated app-file runtime.");
Expand Down Expand Up @@ -278,16 +273,15 @@ async function blobToBase64(blob) {

async function deleteThumbnailPathIfExists(path) {
const runtime = ensureThumbnailRuntime();

try {
await runtime.api.fileDelete(path);
} catch (error) {
if (isNotFoundError(error)) {
return;
}

throw error;
}
// Idempotent delete: pass `ifExists: true` so the server returns 200
// and lists the path under `skipped` instead of throwing 404 when the
// alternative-format thumbnail file (e.g. the `thumbnail.jpg` sibling
// of a `thumbnail.webp` that has always existed alone) was never on
// disk. Without this, every widget move generates a 404 entry in the
// DevTools console because the browser logs failed responses before
// JavaScript can suppress them, and triggers the fetch-proxy retry
// path which is what surfaces as a UI hitch during rapid moves.
await runtime.api.fileDelete(path, { ifExists: true });
}

export function buildSpaceThumbnailPath(spaceId, fileName = SPACE_THUMBNAIL_WEBP_FILE) {
Expand Down
1 change: 1 addition & 0 deletions server/api/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_delete` defaults to strict semantics (404 when the path is gone) so user-initiated deletes can surface a real "this resource is gone" diagnostic; cache-cleanup callers can pass `ifExists: true` in the body to opt into RFC 7231 idempotent delete, in which case missing paths are returned in `skipped` instead of throwing 404
- 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/<user>/` 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
Expand Down
7 changes: 7 additions & 0 deletions server/api/file_delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,17 @@ async function handleDelete(context) {
headers: context.headers,
requestUrl: context.requestUrl
});
// `ifExists: true` opts into RFC 7231 idempotent DELETE semantics:
// missing paths resolve to a 200 response with the path listed under
// `skipped` instead of throwing 404. Strict callers (default) keep
// their authoritative 404 so user-initiated deletes can still surface
// a "this resource is gone" diagnostic to the UI.
const ifExists = payload.ifExists === true;

try {
return await runTrackedMutation(context, async () => {
const options = {
ifExists,
maxLayer,
path: readPath(context),
paths: payload.paths,
Expand Down
76 changes: 60 additions & 16 deletions server/lib/customware/file_access.js
Original file line number Diff line number Diff line change
Expand Up @@ -1309,8 +1309,15 @@ function normalizeDeleteRequests(options = {}) {
runtimeParams: options.runtimeParams,
username: options.username
});
// Per RFC 7231 DELETE is idempotent. Callers that want that idempotency
// pass `ifExists: true`, in which case paths that resolve to nothing on
// disk are recorded as `skipped` and not reported as 404. Strict callers
// (default) still get the authoritative 404 so user-initiated deletes
// surface a real "this resource is gone" diagnostic.
const ifExists = options.ifExists === true;
const entries = normalizeDeleteEntries(options);
const requests = entries.map((entry) => {
const skipped = [];
const requests = entries.flatMap((entry) => {
const request = isPlainObject(entry) ? entry : { path: entry };
const requestedPath = String(request.path || "").trim();

Expand All @@ -1324,13 +1331,18 @@ function normalizeDeleteRequests(options = {}) {
);

if (!resolvedPath.projectPath || !resolvedPath.exists) {
if (ifExists) {
skipped.push(requestedPath);
return [];
}

throw createHttpError(`Path not found: ${requestedPath}`, 404);
}

ensurePublicAppProjectPath(resolvedPath.projectPath);
ensureWritableProjectPath(resolvedPath.projectPath, accessController);

return {
return [{
absolutePath: createAbsolutePath(
String(options.projectRoot || ""),
resolvedPath.projectPath,
Expand All @@ -1339,7 +1351,7 @@ function normalizeDeleteRequests(options = {}) {
isDirectory: resolvedPath.isDirectory,
path: toAppRelativePath(resolvedPath.projectPath),
projectPath: resolvedPath.projectPath
};
}];
});

requests.forEach((request, index) => {
Expand All @@ -1360,19 +1372,28 @@ function normalizeDeleteRequests(options = {}) {
});
});

return requests;
return { requests, skipped };
}

function deleteAppPaths(options = {}) {
const requests = normalizeDeleteRequests(options);
const { requests, skipped } = normalizeDeleteRequests(options);
const quotaDeltas = getDeleteQuotaDeltas(options, requests);
const quotaPlan = createQuotaPlan(options, quotaDeltas);
// With `ifExists: true` the caller has already accepted that the target
// may be gone, so 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.rmSync` would otherwise throw `ENOENT` and
// surface as a 500. `force: true` makes that case the same no-op the
// pathIndex-says-missing branch already handles in normalizeDeleteRequests.
// Strict callers keep `force: false` so they still get the authoritative
// "this resource is gone" signal.
const rmSyncForce = options.ifExists === true;
let paths;

try {
paths = requests.map((request) => {
fs.rmSync(request.absolutePath, {
force: false,
force: rmSyncForce,
recursive: request.isDirectory
});
return request.path;
Expand All @@ -1384,24 +1405,47 @@ function deleteAppPaths(options = {}) {

applyUserFolderQuotaPlan(quotaPlan);

recordAppPathMutations(
{
projectRoot: options.projectRoot,
quotaCacheUpdated: true,
runtimeParams: options.runtimeParams
},
requests.map((request) => request.projectPath)
);
// Gate on `paths.length`, not `requests.length`: the invariant we want is
// "at least one path mutated on disk". `paths` is set after `fs.rmSync`
// succeeds, so it is the authoritative count even when the loop above
// throws partway through (the catch re-throws and we never reach here).
if (paths.length > 0) {
recordAppPathMutations(
{
projectRoot: options.projectRoot,
quotaCacheUpdated: true,
runtimeParams: options.runtimeParams
},
requests.map((request) => request.projectPath)
);
}

return {
const result = {
count: paths.length,
paths
};

if (skipped.length > 0) {
result.skipped = skipped;
}

return result;
}

function deleteAppPath(options = {}) {
const result = deleteAppPaths(options);
// Match the singular-form contract: the legacy result had a single
// `path` field. With ifExists, a missing path resolves to skipped so
// `paths[0]` is undefined; surface that case explicitly.
if (result.paths.length === 0 && Array.isArray(result.skipped) && result.skipped.length > 0) {
return {
path: null,
skipped: result.skipped
};
}

return {
path: deleteAppPaths(options).paths[0]
path: result.paths[0]
};
}

Expand Down