From 16bb6b663d22a1a1db1b71d2a7bfc9182cfe3e8e Mon Sep 17 00:00:00 2001 From: Joe Licata Date: Fri, 8 May 2026 23:12:37 +0000 Subject: [PATCH 1/2] refactor: Rename session_id to storage_session_id across API and models Updated the API responses and models to replace all instances of session_id with storage_session_id for consistency. This change includes updates to the upload, list, and batch file operations, as well as adjustments in the corresponding tests and documentation to reflect the new naming convention. --- src/api/files.py | 3 ++ src/models/exec.py | 19 ++++++++-- src/models/programmatic.py | 12 ++++-- src/services/orchestrator.py | 3 +- tests/functional/test_client_replay.py | 10 ++--- tests/functional/test_concurrent_file_exec.py | 4 +- tests/functional/test_exec_workflow.py | 6 +-- tests/functional/test_files.py | 34 ++++++++--------- tests/functional/test_generated_artifacts.py | 2 +- tests/functional/test_mounted_file_edits.py | 16 ++++---- tests/functional/test_ptc.py | 4 +- tests/functional/test_timing.py | 2 +- tests/integration/test_api_contracts.py | 6 +-- tests/integration/test_exec_api.py | 2 +- tests/integration/test_librechat_compat.py | 37 ++++++++++--------- tests/integration/test_programmatic_api.py | 2 +- tests/integration/test_session_behavior.py | 8 ++-- tests/snapshots/exec_response_python.json | 3 +- tests/snapshots/file_upload_response.json | 35 ++++++------------ tests/unit/test_exec_models.py | 30 +++++++++++++-- tests/unit/test_programmatic.py | 2 +- 21 files changed, 140 insertions(+), 100 deletions(-) diff --git a/src/api/files.py b/src/api/files.py index 59f85fa..8b42e09 100644 --- a/src/api/files.py +++ b/src/api/files.py @@ -172,6 +172,7 @@ async def upload_file( # Note: Production API returns different format with fileId instead of id return { "message": "success", + "storage_session_id": session_id, "session_id": session_id, "files": [ {"filename": file["name"], "fileId": file["id"]} @@ -339,6 +340,7 @@ async def upload_files_batch( return { "message": message, + "storage_session_id": session_id, "session_id": session_id, "files": results, "succeeded": succeeded, @@ -408,6 +410,7 @@ async def list_files( { "name": f"{session_id}/{file_info.file_id}", "id": file_info.file_id, + "storage_session_id": session_id, "session_id": session_id, "content": None, # Not returned in list "size": file_info.size, diff --git a/src/models/exec.py b/src/models/exec.py index dae1d20..88ab861 100644 --- a/src/models/exec.py +++ b/src/models/exec.py @@ -5,26 +5,37 @@ from typing import Dict, List, Optional, Any # Third-party imports -from pydantic import BaseModel, Field +from pydantic import AliasChoices, BaseModel, ConfigDict, Field, computed_field class FileRef(BaseModel): """File reference model for execution response.""" + model_config = ConfigDict(populate_by_name=True) + id: str name: str - path: Optional[str] = None # Make path optional - session_id: Optional[str] = None # Session ID for cross-message file persistence + path: Optional[str] = None + session_id: Optional[str] = None inherited: Optional[bool] = None entity_id: Optional[str] = None modified_from: Optional[Dict[str, str]] = None + @computed_field # type: ignore[prop-decorator] + @property + def storage_session_id(self) -> Optional[str]: + return self.session_id + class RequestFile(BaseModel): """Request file model.""" + model_config = ConfigDict(populate_by_name=True) + id: str - session_id: str + session_id: str = Field( + validation_alias=AliasChoices("storage_session_id", "session_id"), + ) name: str entity_id: Optional[str] = None diff --git a/src/models/programmatic.py b/src/models/programmatic.py index 7a0208c..b7893cd 100644 --- a/src/models/programmatic.py +++ b/src/models/programmatic.py @@ -8,7 +8,7 @@ from typing import Any, Dict, List, Optional -from pydantic import BaseModel, Field, validator +from pydantic import AliasChoices, BaseModel, ConfigDict, Field, validator SUPPORTED_PTC_LANGUAGES = {"py", "bash"} @@ -51,12 +51,18 @@ class PTCFileInput(BaseModel): """File payload for PTC initial execution. Matches the LibreChat/librechat-agents CodeEnvFile shape: - {session_id, id, name} + {storage_session_id, id, name} """ + model_config = ConfigDict(populate_by_name=True) + id: str = Field(..., description="File identifier") name: str = Field(..., description="Original filename for the referenced file") - session_id: str = Field(..., description="Source session for a referenced file") + session_id: str = Field( + ..., + description="Source session for a referenced file", + validation_alias=AliasChoices("storage_session_id", "session_id"), + ) class ProgrammaticExecRequest(BaseModel): diff --git a/src/services/orchestrator.py b/src/services/orchestrator.py index afb64c2..56a5fac 100644 --- a/src/services/orchestrator.py +++ b/src/services/orchestrator.py @@ -786,7 +786,8 @@ async def _handle_generated_files(self, ctx: ExecutionContext) -> List[FileRef]: if meta.get("modified_from_id"): file_ref.modified_from = { "id": meta["modified_from_id"], - "session_id": meta.get("modified_from_session_id") or "", + "storage_session_id": meta.get("modified_from_session_id") + or "", } generated.append(file_ref) logger.debug( diff --git a/tests/functional/test_client_replay.py b/tests/functional/test_client_replay.py index 187eefd..cd39a92 100644 --- a/tests/functional/test_client_replay.py +++ b/tests/functional/test_client_replay.py @@ -12,7 +12,7 @@ def _normalize_artifact_files(result: dict) -> list[dict]: session_id = result["session_id"] return [ { - "session_id": file_info.get("session_id") or session_id, + "storage_session_id": file_info.get("storage_session_id") or session_id, "id": file_info["id"], "name": file_info["name"], } @@ -36,7 +36,7 @@ async def _fetch_runtime_file_refs( file_id = name_parts[1].split(".")[0] if len(name_parts) > 1 else "" file_references.append( { - "session_id": session_id, + "storage_session_id": session_id, "id": file_id, "name": file_info["metadata"]["original-filename"], } @@ -128,7 +128,7 @@ async def test_uploaded_files_follow_runtime_session_when_first_exec_has_no_outp data={"entity_id": unique_entity_id}, ) assert upload.status_code == 200, upload.text - upload_session_id = upload.json()["session_id"] + upload_session_id = upload.json()["storage_session_id"] upload_refs = await _fetch_runtime_file_refs( async_client, auth_headers, upload_session_id ) @@ -164,7 +164,7 @@ async def test_uploaded_files_survive_runtime_fallback_after_outputs_are_generat data={"entity_id": unique_entity_id}, ) assert upload.status_code == 200, upload.text - upload_session_id = upload.json()["session_id"] + upload_session_id = upload.json()["storage_session_id"] upload_refs = await _fetch_runtime_file_refs( async_client, auth_headers, upload_session_id ) @@ -298,7 +298,7 @@ async def test_ptc_fallback_refs_preserve_files_after_continuation( ) assert upload.status_code == 200, upload.text upload_result = upload.json() - session_id = upload_result["session_id"] + session_id = upload_result["storage_session_id"] initial = await _start_ptc_like_runtime( async_client, diff --git a/tests/functional/test_concurrent_file_exec.py b/tests/functional/test_concurrent_file_exec.py index 8e3d3a0..c7250df 100644 --- a/tests/functional/test_concurrent_file_exec.py +++ b/tests/functional/test_concurrent_file_exec.py @@ -56,7 +56,7 @@ async def test_large_file_exec_does_not_block_concurrent_requests( assert upload_resp.status_code == 200, f"Upload failed: {upload_resp.text}" result = upload_resp.json() - session_id = result["session_id"] + session_id = result["storage_session_id"] file_id = result["files"][0]["fileId"] filename = result["files"][0]["filename"] @@ -72,7 +72,7 @@ async def exec_with_file(idx: int) -> tuple: "lang": "py", "session_id": session_id, "files": [ - {"id": file_id, "session_id": session_id, "name": filename} + {"id": file_id, "storage_session_id": session_id, "name": filename} ], }, timeout=60.0, diff --git a/tests/functional/test_exec_workflow.py b/tests/functional/test_exec_workflow.py index 9e4c5ed..b04830c 100644 --- a/tests/functional/test_exec_workflow.py +++ b/tests/functional/test_exec_workflow.py @@ -172,7 +172,7 @@ async def test_file_ref_does_not_leak_session_across_users( ) assert upload.status_code == 200 upload_data = upload.json() - upload_session = upload_data["session_id"] + upload_session = upload_data["storage_session_id"] file_id = upload_data["files"][0]["fileId"] filename = upload_data["files"][0]["filename"] @@ -187,7 +187,7 @@ async def test_file_ref_does_not_leak_session_across_users( "files": [ { "id": file_id, - "session_id": upload_session, + "storage_session_id": upload_session, "name": filename, } ], @@ -208,7 +208,7 @@ async def test_file_ref_does_not_leak_session_across_users( "files": [ { "id": file_id, - "session_id": upload_session, + "storage_session_id": upload_session, "name": filename, } ], diff --git a/tests/functional/test_files.py b/tests/functional/test_files.py index 6e057f6..e87682c 100644 --- a/tests/functional/test_files.py +++ b/tests/functional/test_files.py @@ -25,7 +25,7 @@ async def test_upload_single_file( result = response.json() assert result["message"] == "success" - assert "session_id" in result + assert "storage_session_id" in result assert len(result["files"]) == 1 assert "fileId" in result["files"][0] assert "filename" in result["files"][0] @@ -50,10 +50,10 @@ async def test_librechat_upload_format( assert response.json()["message"] == "success" @pytest.mark.asyncio - async def test_upload_returns_session_id( + async def test_upload_returns_storage_session_id( self, async_client, auth_headers, unique_entity_id ): - """Upload response includes session_id.""" + """Upload response includes storage_session_id.""" files = {"files": ("test.txt", b"content", "text/plain")} data = {"entity_id": unique_entity_id} @@ -65,8 +65,8 @@ async def test_upload_returns_session_id( ) result = response.json() - assert "session_id" in result - assert len(result["session_id"]) > 0 + assert "storage_session_id" in result + assert len(result["storage_session_id"]) > 0 @pytest.mark.asyncio async def test_upload_returns_file_info( @@ -120,7 +120,7 @@ async def test_list_files_after_upload( files=files, data={"entity_id": unique_entity_id}, ) - session_id = upload.json()["session_id"] + session_id = upload.json()["storage_session_id"] # List files response = await async_client.get( @@ -145,7 +145,7 @@ async def test_list_files_detail_simple( files=files, data={"entity_id": unique_entity_id}, ) - session_id = upload.json()["session_id"] + session_id = upload.json()["storage_session_id"] # List with simple detail response = await async_client.get( @@ -170,7 +170,7 @@ async def test_list_files_detail_summary( files=files, data={"entity_id": unique_entity_id}, ) - session_id = upload.json()["session_id"] + session_id = upload.json()["storage_session_id"] # List with summary detail response = await async_client.get( @@ -204,7 +204,7 @@ async def test_detail_full_has_original_filename_metadata( data={"entity_id": unique_entity_id}, ) assert upload.status_code == 200 - session_id = upload.json()["session_id"] + session_id = upload.json()["storage_session_id"] # Get full detail response = await async_client.get( @@ -237,7 +237,7 @@ async def test_detail_full_has_required_fields( files=files, data={"entity_id": unique_entity_id}, ) - session_id = upload.json()["session_id"] + session_id = upload.json()["storage_session_id"] response = await async_client.get( f"/files/{session_id}?detail=full", @@ -278,7 +278,7 @@ async def test_download_uploaded_file( data={"entity_id": unique_entity_id}, ) - session_id = upload.json()["session_id"] + session_id = upload.json()["storage_session_id"] file_id = upload.json()["files"][0]["fileId"] response = await async_client.get( @@ -322,7 +322,7 @@ async def test_uploaded_file_readable_at_mnt_data( ) assert upload.status_code == 200 upload_data = upload.json() - session_id = upload_data["session_id"] + session_id = upload_data["storage_session_id"] file_id = upload_data["files"][0]["fileId"] filename = upload_data["files"][0]["filename"] @@ -341,7 +341,7 @@ async def test_uploaded_file_readable_at_mnt_data( ), "lang": "py", "session_id": session_id, - "files": [{"id": file_id, "session_id": session_id, "name": filename}], + "files": [{"id": file_id, "storage_session_id": session_id, "name": filename}], }, ) @@ -366,7 +366,7 @@ async def test_uploaded_file_readable_via_relative_path( data={"entity_id": unique_entity_id}, ) upload_data = upload.json() - session_id = upload_data["session_id"] + session_id = upload_data["storage_session_id"] file_id = upload_data["files"][0]["fileId"] filename = upload_data["files"][0]["filename"] @@ -377,7 +377,7 @@ async def test_uploaded_file_readable_via_relative_path( "code": f"print(open('{filename}').read())", "lang": "py", "session_id": session_id, - "files": [{"id": file_id, "session_id": session_id, "name": filename}], + "files": [{"id": file_id, "storage_session_id": session_id, "name": filename}], }, ) @@ -400,7 +400,7 @@ async def test_upload_execute_generate_download( data={"entity_id": unique_entity_id}, ) upload_data = upload.json() - session_id = upload_data["session_id"] + session_id = upload_data["storage_session_id"] file_id = upload_data["files"][0]["fileId"] filename = upload_data["files"][0]["filename"] @@ -424,7 +424,7 @@ async def test_upload_execute_generate_download( ), "lang": "py", "session_id": session_id, - "files": [{"id": file_id, "session_id": session_id, "name": filename}], + "files": [{"id": file_id, "storage_session_id": session_id, "name": filename}], }, ) diff --git a/tests/functional/test_generated_artifacts.py b/tests/functional/test_generated_artifacts.py index 6ae02c6..d173a58 100644 --- a/tests/functional/test_generated_artifacts.py +++ b/tests/functional/test_generated_artifacts.py @@ -114,7 +114,7 @@ async def test_generated_file_is_reused_on_follow_up_execution( "files": [ { "id": generated_file["id"], - "session_id": generate_result["session_id"], + "storage_session_id": generate_result["session_id"], "name": generated_file["name"], } ], diff --git a/tests/functional/test_mounted_file_edits.py b/tests/functional/test_mounted_file_edits.py index 32089af..75e0891 100644 --- a/tests/functional/test_mounted_file_edits.py +++ b/tests/functional/test_mounted_file_edits.py @@ -31,7 +31,7 @@ async def test_overwrite_mounted_file_persists(self, async_client, auth_headers) ) assert upload.status_code == 200, upload.text upload_result = upload.json() - session_id = upload_result["session_id"] + session_id = upload_result["storage_session_id"] file_id = upload_result["files"][0]["fileId"] execute = await async_client.post( @@ -46,7 +46,7 @@ async def test_overwrite_mounted_file_persists(self, async_client, auth_headers) ), "session_id": session_id, "files": [ - {"id": file_id, "session_id": session_id, "name": "test.txt"} + {"id": file_id, "storage_session_id": session_id, "name": "test.txt"} ], }, ) @@ -78,7 +78,7 @@ async def test_append_to_mounted_file_persists(self, async_client, auth_headers) ) assert upload.status_code == 200, upload.text upload_result = upload.json() - session_id = upload_result["session_id"] + session_id = upload_result["storage_session_id"] file_id = upload_result["files"][0]["fileId"] execute = await async_client.post( @@ -93,7 +93,7 @@ async def test_append_to_mounted_file_persists(self, async_client, auth_headers) "print('Appended')\n" ), "session_id": session_id, - "files": [{"id": file_id, "session_id": session_id, "name": "log.txt"}], + "files": [{"id": file_id, "storage_session_id": session_id, "name": "log.txt"}], }, ) assert execute.status_code == 200, execute.text @@ -127,7 +127,7 @@ async def test_delete_mounted_file_does_not_error( ) assert upload.status_code == 200, upload.text upload_result = upload.json() - session_id = upload_result["session_id"] + session_id = upload_result["storage_session_id"] file_id = upload_result["files"][0]["fileId"] execute = await async_client.post( @@ -142,7 +142,7 @@ async def test_delete_mounted_file_does_not_error( "print('File deleted')\n" ), "files": [ - {"id": file_id, "session_id": session_id, "name": "temp.txt"} + {"id": file_id, "storage_session_id": session_id, "name": "temp.txt"} ], }, ) @@ -161,7 +161,7 @@ async def test_edit_csv_file_persists(self, async_client, auth_headers): ) assert upload.status_code == 200, upload.text upload_result = upload.json() - session_id = upload_result["session_id"] + session_id = upload_result["storage_session_id"] file_id = upload_result["files"][0]["fileId"] execute = await async_client.post( @@ -178,7 +178,7 @@ async def test_edit_csv_file_persists(self, async_client, auth_headers): ), "session_id": session_id, "files": [ - {"id": file_id, "session_id": session_id, "name": "data.csv"} + {"id": file_id, "storage_session_id": session_id, "name": "data.csv"} ], }, ) diff --git a/tests/functional/test_ptc.py b/tests/functional/test_ptc.py index f1bcaa3..7edab99 100644 --- a/tests/functional/test_ptc.py +++ b/tests/functional/test_ptc.py @@ -95,7 +95,7 @@ async def test_ptc_accepts_referenced_session_files( assert upload.status_code == 200 upload_payload = upload.json() - session_id = upload_payload["session_id"] + session_id = upload_payload["storage_session_id"] file_id = upload_payload["files"][0]["fileId"] response = await async_client.post( @@ -109,7 +109,7 @@ async def test_ptc_accepts_referenced_session_files( "tools": [], "files": [ { - "session_id": session_id, + "storage_session_id": session_id, "id": file_id, "name": "report.csv", } diff --git a/tests/functional/test_timing.py b/tests/functional/test_timing.py index 4567dde..9f91d34 100644 --- a/tests/functional/test_timing.py +++ b/tests/functional/test_timing.py @@ -100,7 +100,7 @@ async def test_download_under_5s( files=files, data={"entity_id": unique_entity_id}, ) - session_id = upload.json()["session_id"] + session_id = upload.json()["storage_session_id"] file_id = upload.json()["files"][0]["fileId"] # Time the download diff --git a/tests/integration/test_api_contracts.py b/tests/integration/test_api_contracts.py index 59f1221..d0cbdc4 100644 --- a/tests/integration/test_api_contracts.py +++ b/tests/integration/test_api_contracts.py @@ -402,10 +402,10 @@ def test_upload_without_entity_id(self, client, auth_headers, mock_file_service) assert response.status_code == 200 data = response.json() - # session_id is at the top level of the response - assert "session_id" in data + # storage_session_id is at the top level of the response + assert "storage_session_id" in data # API generates a random session ID when no entity_id is provided - assert len(data["session_id"]) > 0 + assert len(data["storage_session_id"]) > 0 class TestFileListContract: diff --git a/tests/integration/test_exec_api.py b/tests/integration/test_exec_api.py index b9c65a1..eabc9cd 100644 --- a/tests/integration/test_exec_api.py +++ b/tests/integration/test_exec_api.py @@ -198,7 +198,7 @@ def test_exec_with_files(self, client, auth_headers, mock_execution_service): "code": "with open('data.txt', 'r') as f: print(f.read())", "lang": "py", "files": [ - {"id": "file-123", "session_id": "test-session", "name": "data.txt"} + {"id": "file-123", "storage_session_id": "test-session", "name": "data.txt"} ], } diff --git a/tests/integration/test_librechat_compat.py b/tests/integration/test_librechat_compat.py index 093e96e..3d7fc6f 100644 --- a/tests/integration/test_librechat_compat.py +++ b/tests/integration/test_librechat_compat.py @@ -108,7 +108,7 @@ def test_librechat_request_with_files( "files": [ { "id": "file-svc-abc123", - "session_id": "sess_xyz789", + "storage_session_id": "sess_xyz789", "name": "data.csv", } ], @@ -128,9 +128,9 @@ def test_librechat_request_with_multiple_files( "code": "import os; print(os.listdir('.'))", "lang": "py", "files": [ - {"id": "file-1", "session_id": "sess-1", "name": "file1.txt"}, - {"id": "file-2", "session_id": "sess-2", "name": "file2.txt"}, - {"id": "file-3", "session_id": "sess-3", "name": "file3.csv"}, + {"id": "file-1", "storage_session_id": "sess-1", "name": "file1.txt"}, + {"id": "file-2", "storage_session_id": "sess-2", "name": "file2.txt"}, + {"id": "file-3", "storage_session_id": "sess-3", "name": "file3.csv"}, ], } @@ -384,19 +384,19 @@ def test_multipart_upload_format(self, client, auth_headers): assert response.status_code == 200 result = response.json() - # API returns {message, session_id, files: [{fileId, filename}]} + # API returns {message, storage_session_id, files: [{fileId, filename}]} # LibreChat checks: if (result.message !== 'success') throw error assert result.get("message") == "success", "LibreChat expects message='success'" assert "files" in result assert len(result["files"]) == 1 - assert "session_id" in result + assert "storage_session_id" in result file_info = result["files"][0] assert "fileId" in file_info assert "filename" in file_info - def test_upload_response_has_session_id(self, client, auth_headers): - """Test that upload response includes a session_id.""" + def test_upload_response_has_storage_session_id(self, client, auth_headers): + """Test that upload response includes a storage_session_id.""" entity_id = "asst_specific_entity" # LibreChat uses 'file' (singular) files = {"file": ("test.txt", io.BytesIO(b"content"), "text/plain")} @@ -405,9 +405,8 @@ def test_upload_response_has_session_id(self, client, auth_headers): response = client.post("/upload", files=files, data=data, headers=auth_headers) result = response.json() - # API generates a new session_id for uploads (entity_id is currently not used) - assert "session_id" in result - assert len(result["session_id"]) > 0 + assert "storage_session_id" in result + assert len(result["storage_session_id"]) > 0 def test_librechat_upload_with_user_id_header(self, client, auth_headers): """ @@ -722,7 +721,7 @@ def test_upload_then_check_summary(self, client, auth_headers): assert upload_response.status_code == 200 upload_result = upload_response.json() assert upload_result["message"] == "success" - session_id = upload_result["session_id"] + session_id = upload_result["storage_session_id"] file_id = upload_result["files"][0]["fileId"] # Step 2: Check summary endpoint @@ -769,7 +768,7 @@ def test_upload_then_exec_with_file_ref(self, mock_execute, client, auth_headers ) assert upload_response.status_code == 200 upload_result = upload_response.json() - session_id = upload_result["session_id"] + session_id = upload_result["storage_session_id"] file_id = upload_result["files"][0]["fileId"] # Step 2: Execute with file reference @@ -786,7 +785,11 @@ def test_upload_then_exec_with_file_ref(self, mock_execute, client, auth_headers "code": "with open('/mnt/data/input.txt') as f: print(f.read())", "lang": "py", "files": [ - {"id": file_id, "session_id": session_id, "name": "input.txt"} + { + "id": file_id, + "storage_session_id": session_id, + "name": "input.txt", + } ], }, headers=auth_headers, @@ -953,7 +956,7 @@ def test_prime_files_reupload_flow(self, client, auth_headers): assert upload_response.status_code == 200 result = upload_response.json() assert result["message"] == "success" - assert "session_id" in result + assert "storage_session_id" in result assert len(result["files"]) == 1 def test_prime_files_empty_session_returns_empty_array(self, client, auth_headers): @@ -1702,7 +1705,7 @@ def test_ptc_initial_request_accepts_code_env_file_shape( "code": "print('Hello from PTC')", "files": [ { - "session_id": "upload-session", + "storage_session_id": "upload-session", "id": "file-123", "name": "report.csv", } @@ -1944,7 +1947,7 @@ def test_response_shape_matches_librechat_contract(self, client, auth_headers): assert response.status_code == 200 result = response.json() # All five top-level keys present (LibreChat reads each one) - for key in ("message", "session_id", "files", "succeeded", "failed"): + for key in ("message", "storage_session_id", "files", "succeeded", "failed"): assert key in result, f"Missing required key: {key}" assert result["message"] == "success" assert result["succeeded"] == 2 diff --git a/tests/integration/test_programmatic_api.py b/tests/integration/test_programmatic_api.py index 811c760..c27daa0 100644 --- a/tests/integration/test_programmatic_api.py +++ b/tests/integration/test_programmatic_api.py @@ -275,7 +275,7 @@ def test_initial_request_accepts_librechat_file_refs( "code": "print('hello')", "files": [ { - "session_id": "upload-session", + "storage_session_id": "upload-session", "id": "file-123", "name": "report.csv", } diff --git a/tests/integration/test_session_behavior.py b/tests/integration/test_session_behavior.py index effa95e..5f85202 100644 --- a/tests/integration/test_session_behavior.py +++ b/tests/integration/test_session_behavior.py @@ -415,7 +415,9 @@ def test_uploaded_file_available_in_execution(self, client, auth_headers): "/files/upload", files=files, data=data, headers=auth_headers ) assert upload_response.status_code == 200 - uploaded_file = upload_response.json()["files"][0] + upload_result = upload_response.json() + uploaded_file = upload_result["files"][0] + storage_session_id = upload_result["storage_session_id"] # Execute code that references the file exec_response = client.post( @@ -426,8 +428,8 @@ def test_uploaded_file_available_in_execution(self, client, auth_headers): "entity_id": "file-test-entity", "files": [ { - "id": uploaded_file["id"], - "session_id": uploaded_file["session_id"], + "id": uploaded_file["fileId"], + "storage_session_id": storage_session_id, "name": "data.txt", } ], diff --git a/tests/snapshots/exec_response_python.json b/tests/snapshots/exec_response_python.json index e7b37b1..506e1c1 100644 --- a/tests/snapshots/exec_response_python.json +++ b/tests/snapshots/exec_response_python.json @@ -29,7 +29,8 @@ "file_ref_schema": { "id": "string (required, file identifier)", "name": "string (required, filename)", - "path": "string (optional, file path in container)" + "path": "string (optional, file path in container)", + "storage_session_id": "string (optional, S3 storage session for this file)" }, "notes": [ diff --git a/tests/snapshots/file_upload_response.json b/tests/snapshots/file_upload_response.json index 9661d93..ba6e44d 100644 --- a/tests/snapshots/file_upload_response.json +++ b/tests/snapshots/file_upload_response.json @@ -1,44 +1,33 @@ { - "_description": "Expected /files/upload response format", - "_version": "1.0.0", + "_description": "Expected /upload response format", + "_version": "2.0.0", "_librechat_compatible": true, "request": { "method": "POST", - "endpoint": "/files/upload", + "endpoint": "/upload", "content_type": "multipart/form-data", "fields": { - "files": "binary file(s)", - "entity_id": "optional session/entity identifier" + "file": "binary file (LibreChat uses singular 'file')", + "entity_id": "optional entity identifier" } }, - "response": { - "files": [ - { - "id": "string (unique file identifier)", - "name": "string (original filename)", - "size": "integer (file size in bytes)", - "session_id": "string (session the file belongs to)" - } - ] - }, - "response_example": { + "message": "success", + "storage_session_id": "a1b2c3d4-e5f6-7890-abcd-ef1234567890", "files": [ { - "id": "file-svc-abc123def456", - "name": "data.csv", - "size": 1024, - "session_id": "entity-xyz789" + "fileId": "file-svc-abc123def456", + "filename": "data.csv" } ] }, "notes": [ - "session_id matches entity_id if provided", - "session_id is 'temp-session' if no entity_id", - "Multiple files can be uploaded in single request", + "storage_session_id is the S3 bucket prefix where files are stored", + "LibreChat reads result.storage_session_id (renamed from session_id in Phase C)", + "Multiple files can be uploaded via /upload/batch endpoint", "File ID format is generated by the service" ] } diff --git a/tests/unit/test_exec_models.py b/tests/unit/test_exec_models.py index e873bff..a5bf506 100644 --- a/tests/unit/test_exec_models.py +++ b/tests/unit/test_exec_models.py @@ -29,6 +29,7 @@ def test_inherited_true_serializes(self): assert dumped["entity_id"] == "agent-1" assert dumped["id"] == "orig-1" assert dumped["session_id"] == "sess-1" + assert dumped["storage_session_id"] == "sess-1" def test_inherited_none_excluded_with_exclude_none(self): ref = FileRef(id="fid", name="out.png", session_id="sess-1") @@ -36,18 +37,22 @@ def test_inherited_none_excluded_with_exclude_none(self): assert "inherited" not in dumped assert "entity_id" not in dumped assert "modified_from" not in dumped - # Existing optional fields must also be excluded. assert "path" not in dumped + assert dumped["session_id"] == "sess-1" + assert dumped["storage_session_id"] == "sess-1" def test_modified_from_preserved(self): ref = FileRef( id="new-fid", name="report.csv", session_id="sess-2", - modified_from={"id": "old-fid", "session_id": "sess-1"}, + modified_from={"id": "old-fid", "storage_session_id": "sess-1"}, ) dumped = ref.model_dump(exclude_none=True) - assert dumped["modified_from"] == {"id": "old-fid", "session_id": "sess-1"} + assert dumped["modified_from"] == { + "id": "old-fid", + "storage_session_id": "sess-1", + } class TestRequestFileEntityId: @@ -67,6 +72,25 @@ def test_entity_id_optional(self): assert rf.entity_id is None +class TestStorageSessionIdAlias: + """RequestFile accepts storage_session_id (new) and session_id (legacy). + FileRef serializes session_id as storage_session_id.""" + + def test_request_file_accepts_storage_session_id(self): + rf = RequestFile(id="fid", storage_session_id="sess", name="data.csv") + assert rf.session_id == "sess" + + def test_request_file_accepts_legacy_session_id(self): + rf = RequestFile(id="fid", session_id="sess", name="data.csv") + assert rf.session_id == "sess" + + def test_fileref_emits_both_session_id_and_storage_session_id(self): + ref = FileRef(id="fid", name="out.png", session_id="sess-1") + dumped = ref.model_dump(exclude_none=True) + assert dumped["storage_session_id"] == "sess-1" + assert dumped["session_id"] == "sess-1" + + class TestExecRequestTimeout: """ExecRequest.timeout: optional, milliseconds, range 1000-300000.""" diff --git a/tests/unit/test_programmatic.py b/tests/unit/test_programmatic.py index 644b33e..c885218 100644 --- a/tests/unit/test_programmatic.py +++ b/tests/unit/test_programmatic.py @@ -214,7 +214,7 @@ def test_initial_request_with_all_fields(self): timeout=60000, files=[ { - "session_id": "source-session", + "storage_session_id": "source-session", "id": "file-123", "name": "test.txt", } From 7567f3e5e88ef4a805fb4e5936baf163e18a237e Mon Sep 17 00:00:00 2001 From: Joe Licata Date: Wed, 13 May 2026 14:03:50 +0000 Subject: [PATCH 2/2] feat: Enhance file upload and listing with resource kind and version support Updated the upload_files_batch and list_files functions to include support for resource kind and version parameters. The upload logic now marks files as agent files based on the kind provided. Additionally, the RequestFile and FileRef models have been updated to accept resource_id, kind, and version fields, with corresponding tests added to validate these changes. --- src/api/files.py | 17 +++++++- src/models/exec.py | 6 +++ src/models/programmatic.py | 3 ++ tests/integration/test_librechat_compat.py | 44 +++++++++++++++++++++ tests/unit/test_exec_models.py | 46 ++++++++++++++++++++++ 5 files changed, 115 insertions(+), 1 deletion(-) diff --git a/src/api/files.py b/src/api/files.py index 8b42e09..2d03e52 100644 --- a/src/api/files.py +++ b/src/api/files.py @@ -250,7 +250,10 @@ async def upload_files_batch( entity_id: Optional[str] = ( entity_id_raw if isinstance(entity_id_raw, str) and entity_id_raw else None ) - is_agent_file = entity_id is not None + kind_raw = form.get("kind") + is_agent_file = entity_id is not None or ( + isinstance(kind_raw, str) and kind_raw in ("skill", "agent") + ) read_only_raw = form.get("read_only") is_read_only = isinstance(read_only_raw, str) and read_only_raw.lower() in ( @@ -355,6 +358,18 @@ async def list_files( None, description="Detail level: 'simple' for basic info, otherwise full details", ), + kind: Optional[str] = Query( + None, + description="Resource kind filter: 'skill', 'agent', or 'user'", + ), + id: Optional[str] = Query( + None, + description="Resource id for scoped file listing", + ), + version: Optional[int] = Query( + None, + description="Resource version (only meaningful when kind=skill)", + ), file_service: FileServiceDep = None, ): """List all files in a session with optional detail parameter - LibreChat compatible.""" diff --git a/src/models/exec.py b/src/models/exec.py index 88ab861..9130167 100644 --- a/src/models/exec.py +++ b/src/models/exec.py @@ -19,6 +19,9 @@ class FileRef(BaseModel): session_id: Optional[str] = None inherited: Optional[bool] = None entity_id: Optional[str] = None + resource_id: Optional[str] = None + kind: Optional[str] = None + version: Optional[int] = None modified_from: Optional[Dict[str, str]] = None @computed_field # type: ignore[prop-decorator] @@ -38,6 +41,9 @@ class RequestFile(BaseModel): ) name: str entity_id: Optional[str] = None + resource_id: Optional[str] = None + kind: Optional[str] = None + version: Optional[int] = None class ExecRequest(BaseModel): diff --git a/src/models/programmatic.py b/src/models/programmatic.py index b7893cd..9051c39 100644 --- a/src/models/programmatic.py +++ b/src/models/programmatic.py @@ -63,6 +63,9 @@ class PTCFileInput(BaseModel): description="Source session for a referenced file", validation_alias=AliasChoices("storage_session_id", "session_id"), ) + resource_id: Optional[str] = None + kind: Optional[str] = None + version: Optional[int] = None class ProgrammaticExecRequest(BaseModel): diff --git a/tests/integration/test_librechat_compat.py b/tests/integration/test_librechat_compat.py index 3d7fc6f..7756ba3 100644 --- a/tests/integration/test_librechat_compat.py +++ b/tests/integration/test_librechat_compat.py @@ -2041,6 +2041,50 @@ def test_nested_filename_preserved_in_response( # The stored filename also preserves the path so S3/sandbox round-trip works. assert "skills/weather_lookup/SKILL.md" in setup_mocks["stored"] + def test_kind_skill_marks_files_as_agent(self, client, auth_headers, setup_mocks): + """kind=skill bypasses extension whitelist for skill-priming uploads.""" + files = [ + ("file", ("schema.xsd", io.BytesIO(b""), "application/xml")) + ] + data = {"kind": "skill", "id": "skill_abc123", "version": "3"} + response = client.post( + "/upload/batch", files=files, data=data, headers=auth_headers + ) + + assert response.status_code == 200 + store = setup_mocks["file_service"].store_uploaded_file + assert store.await_count == 1 + kwargs = store.await_args.kwargs + assert kwargs["is_agent_file"] is True + + def test_kind_agent_marks_files_as_agent(self, client, auth_headers, setup_mocks): + """kind=agent also bypasses extension whitelist.""" + files = [ + ("file", ("config.toml", io.BytesIO(b"[tool]"), "application/toml")) + ] + data = {"kind": "agent", "id": "agent_xyz"} + response = client.post( + "/upload/batch", files=files, data=data, headers=auth_headers + ) + + assert response.status_code == 200 + store = setup_mocks["file_service"].store_uploaded_file + assert store.await_count == 1 + kwargs = store.await_args.kwargs + assert kwargs["is_agent_file"] is True + + def test_batch_response_includes_storage_session_id( + self, client, auth_headers, setup_mocks + ): + """LibreChat validates storage_session_id in batch upload response.""" + files = [("file", ("data.csv", io.BytesIO(b"a,b"), "text/csv"))] + response = client.post("/upload/batch", files=files, headers=auth_headers) + + assert response.status_code == 200 + body = response.json() + assert "storage_session_id" in body + assert body["storage_session_id"] == body["session_id"] + # ============================================================================= # GET /sessions/{session_id}/objects/{file_id} — liveness probe diff --git a/tests/unit/test_exec_models.py b/tests/unit/test_exec_models.py index a5bf506..875acca 100644 --- a/tests/unit/test_exec_models.py +++ b/tests/unit/test_exec_models.py @@ -91,6 +91,52 @@ def test_fileref_emits_both_session_id_and_storage_session_id(self): assert dumped["session_id"] == "sess-1" +class TestCodeEnvFileFields: + """RequestFile and FileRef accept resource_id, kind, and version + fields sent by the librechat-agents CodeEnvFile type.""" + + def test_request_file_accepts_code_env_file_shape(self): + rf = RequestFile( + id="fid", + storage_session_id="sess", + name="data.csv", + resource_id="res-1", + kind="skill", + version=3, + ) + assert rf.session_id == "sess" + assert rf.resource_id == "res-1" + assert rf.kind == "skill" + assert rf.version == 3 + + def test_request_file_code_env_fields_optional(self): + rf = RequestFile(id="fid", session_id="sess", name="data.csv") + assert rf.resource_id is None + assert rf.kind is None + assert rf.version is None + + def test_fileref_resource_id_kind_version(self): + ref = FileRef( + id="fid", + name="out.png", + session_id="sess-1", + resource_id="res-1", + kind="skill", + version=2, + ) + dumped = ref.model_dump(exclude_none=True) + assert dumped["resource_id"] == "res-1" + assert dumped["kind"] == "skill" + assert dumped["version"] == 2 + + def test_fileref_code_env_fields_excluded_when_none(self): + ref = FileRef(id="fid", name="out.png", session_id="sess-1") + dumped = ref.model_dump(exclude_none=True) + assert "resource_id" not in dumped + assert "kind" not in dumped + assert "version" not in dumped + + class TestExecRequestTimeout: """ExecRequest.timeout: optional, milliseconds, range 1000-300000."""