From 0e529249f6b2050992f42386317feab5175d5e20 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 4 Sep 2026 12:25:02 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=AD=20fix:=20Delete=20Code=20Environme?= =?UTF-8?q?nt=20Files=20Through=20the=20Route=20That=20Exists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `deleteCodeEnvFile` tried `/sessions/:sid/objects/:fid` before falling back to `/files/:sid/:fid`. Both have been there since #13424, but only the second is mounted by any released codeapi — the first gained DELETE in LibreChat-AI/code-interpreter#85 — so every deletion paid a guaranteed 404 and a wasted round trip, and #15511 read that 404 as the whole bug. Call `/files/:sid/:fid` directly. It is the safe direction to collapse toward: codeapi has mounted it since its first release, so this works against older deployments as well as post-#85 ones, whereas keeping the other path would not. Collapsing the loop tightens two behaviours that only existed to serve it. A 405 now surfaces instead of being swallowed on the way to a second attempt; there is no second route to try, and a service that refuses the method should say so. A 404 is still treated as "already gone" — that is the only thing it can now mean — but it is logged rather than passed over in silence, because a 404 caused by a misconfigured base URL looks identical and this branch drops the file's metadata record either way. --- api/server/services/Files/Code/crud.js | 70 +++++++++------------ api/server/services/Files/Code/crud.spec.js | 60 ++++++++---------- 2 files changed, 56 insertions(+), 74 deletions(-) diff --git a/api/server/services/Files/Code/crud.js b/api/server/services/Files/Code/crud.js index c7e63540d2..aaa53a82d8 100644 --- a/api/server/services/Files/Code/crud.js +++ b/api/server/services/Files/Code/crud.js @@ -84,7 +84,6 @@ async function deleteCodeEnvFile(req, file) { return; } - const missingOrUnsupportedStatuses = new Set([404, 405]); for (const [executionRouteKey, ref] of refs) { const executionProfile = ref.executionProfile ?? 'default'; const environments = @@ -127,48 +126,37 @@ async function deleteCodeEnvFile(req, file) { const bridgeWorkerId = configuredEnvironment?.workerId ?? configuredEnvironment?.pairing?.workerId; const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); - const baseRequest = { - method: 'delete', - headers: { - 'User-Agent': 'LibreChat/1.0', - ...authHeaders, - ...codeExecutionHeaders({ - executionProfile, - bridgeWorkerId, - }), - }, - httpAgent: codeServerHttpAgent, - httpsAgent: codeServerHttpsAgent, - timeout: 15000, - }; - const urls = [ - `${baseURL}/sessions/${ref.storage_session_id}/objects/${ref.file_id}${query}`, - `${baseURL}/files/${ref.storage_session_id}/${ref.file_id}${query}`, - ]; - - let lastError; - let deleted = false; - for (const url of urls) { - try { - await axios({ ...baseRequest, url }); - deleted = true; - break; - } catch (error) { - lastError = error; - if (!missingOrUnsupportedStatuses.has(error.response?.status)) { - throw error; - } - } - } - if (!deleted && lastError) { - logAxiosError({ - error: lastError, - message: `Error deleting code environment file: ${lastError.message}`, + /* codeapi has mounted DELETE at `/files/:session_id/:fileId` since its + * first release. The file-server's own `/sessions/:id/objects/:fileId` + * only gained DELETE in LibreChat-AI/code-interpreter#85, so trying it + * first cost a guaranteed 404 against every older deployment. */ + try { + await axios({ + method: 'delete', + url: `${baseURL}/files/${ref.storage_session_id}/${ref.file_id}${query}`, + headers: { + 'User-Agent': 'LibreChat/1.0', + ...authHeaders, + ...codeExecutionHeaders({ + executionProfile, + bridgeWorkerId, + }), + }, + httpAgent: codeServerHttpAgent, + httpsAgent: codeServerHttpsAgent, + timeout: 15000, }); - if (lastError.response?.status === 404) { - continue; + } catch (error) { + if (error.response?.status !== 404) { + throw error; } - throw new Error(lastError.message || 'An error occurred during file deletion.'); + /* Already gone. Logged rather than swallowed: a 404 from a + * misconfigured base URL is indistinguishable from one for an absent + * object, and this branch drops the file's record either way. */ + logAxiosError({ + error, + message: `Code environment object already absent: ${error.message}`, + }); } } } diff --git a/api/server/services/Files/Code/crud.spec.js b/api/server/services/Files/Code/crud.spec.js index 4e5c1242db..e35758ee35 100644 --- a/api/server/services/Files/Code/crud.spec.js +++ b/api/server/services/Files/Code/crud.spec.js @@ -212,7 +212,7 @@ describe('Code CRUD', () => { expect(mockAxios).toHaveBeenCalledWith( expect.objectContaining({ method: 'delete', - url: 'https://code-api.example.com/sessions/session-1/objects/file-1?kind=agent&id=agent-abc', + url: 'https://code-api.example.com/files/session-1/file-1?kind=agent&id=agent-abc', headers: expect.objectContaining({ Authorization: 'Bearer codeapi-token', 'User-Agent': 'LibreChat/1.0', @@ -239,7 +239,7 @@ describe('Code CRUD', () => { expect(mockAxios).toHaveBeenCalledWith( expect.objectContaining({ - url: 'https://code-stateful.example.com/sessions/session-1/objects/file-1?kind=agent&id=agent-abc', + url: 'https://code-stateful.example.com/files/session-1/file-1?kind=agent&id=agent-abc', headers: expect.objectContaining({ 'X-CodeAPI-Expected-Profile': 'stateful', }), @@ -270,14 +270,14 @@ describe('Code CRUD', () => { expect(mockAxios).toHaveBeenNthCalledWith( 1, expect.objectContaining({ - url: expect.stringContaining('/sessions/session-1/objects/file-1'), + url: expect.stringContaining('/files/session-1/file-1'), headers: expect.objectContaining({ 'X-CodeAPI-Expected-Profile': 'default' }), }), ); expect(mockAxios).toHaveBeenNthCalledWith( 2, expect.objectContaining({ - url: expect.stringContaining('/sessions/stateful-session/objects/stateful-file'), + url: expect.stringContaining('/files/stateful-session/stateful-file'), headers: expect.objectContaining({ 'X-CodeAPI-Expected-Profile': 'stateful' }), }), ); @@ -320,33 +320,27 @@ describe('Code CRUD', () => { expect(mockAxios).not.toHaveBeenCalled(); }); - it.each([404, 405])( - 'falls back to the legacy code environment delete route after a %s', - async (status) => { - mockAxios - .mockRejectedValueOnce( - Object.assign(new Error('missing route'), { response: { status } }), - ) - .mockResolvedValueOnce({ status: 204 }); + it('never calls the file-server path that only newer codeapi mounts', async () => { + mockAxios.mockResolvedValue({ status: 204 }); - await deleteCodeEnvFile(req, file); + await deleteCodeEnvFile(req, file); - expect(mockAxios).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ - method: 'delete', - url: 'https://code-api.example.com/sessions/session-1/objects/file-1?kind=agent&id=agent-abc', - }), - ); - expect(mockAxios).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ - method: 'delete', - url: 'https://code-api.example.com/files/session-1/file-1?kind=agent&id=agent-abc', - }), - ); - }, - ); + expect(mockAxios).toHaveBeenCalledTimes(1); + expect(mockAxios).not.toHaveBeenCalledWith( + expect.objectContaining({ + url: expect.stringContaining('/sessions/session-1/objects/file-1'), + }), + ); + }); + + it('surfaces an unsupported delete method instead of retrying elsewhere', async () => { + mockAxios.mockRejectedValue( + Object.assign(new Error('method not allowed'), { response: { status: 405 } }), + ); + + await expect(deleteCodeEnvFile(req, file)).rejects.toThrow('method not allowed'); + expect(mockAxios).toHaveBeenCalledTimes(1); + }); it('skips files without a code environment ref', async () => { await deleteCodeEnvFile(req, {}); @@ -356,12 +350,12 @@ describe('Code CRUD', () => { }); it('treats missing code environment objects as already deleted', async () => { - mockAxios - .mockRejectedValueOnce(Object.assign(new Error('missing'), { response: { status: 404 } })) - .mockRejectedValueOnce(Object.assign(new Error('missing'), { response: { status: 404 } })); + mockAxios.mockRejectedValue( + Object.assign(new Error('missing'), { response: { status: 404 } }), + ); await expect(deleteCodeEnvFile(req, file)).resolves.toBeUndefined(); - expect(mockAxios).toHaveBeenCalledTimes(2); + expect(mockAxios).toHaveBeenCalledTimes(1); }); it('throws when code environment deletion fails', async () => {