Fix storage upload pipeline: proper URL encoding, no silent failures

- storage_client: use urllib.parse.quote(path, safe='/') for all URL
  construction, so any filename (spaces, unicode, special chars) works
- storage_client: capture and log Supabase response body on HTTP errors
- storage_client: raise on failure in list_files and remove_files instead
  of returning [] or swallowing errors
- server.py: revert space→underscore hack in _storage_path — URL encoding
  at the HTTP layer is the correct fix
- server.py: wrap cleanup remove_files call so original error is preserved
  even if cleanup also fails

Author: anmolsharma152

codex-backend/server.py

@@ -89,10 +89,9 @@ def verify_thread_ownership(thread_id: str, user_id: str):
 
 
 def _storage_path(user_id: str, filename: str, thread_id: str | None = None) -> str:
-    safe_name = filename.replace(" ", "_")
     if thread_id:
-        return f"{user_id}/{thread_id}/{safe_name}"
-    return f"{user_id}/{safe_name}"
+        return f"{user_id}/{thread_id}/{filename}"
+    return f"{user_id}/{filename}"
 
 
 async def _download_from_storage(storage_path: str, auth_token: str | None = None) -> str | None:
@@ -382,16 +381,18 @@ async def upload_document(file: UploadFile = File(...), current_user=Depends(get
         tmp_path = await _download_from_storage(storage_path, auth_token=current_user.token)
         if not tmp_path:
             raise RuntimeError("Failed to retrieve uploaded file for ingestion")
-        safe_filename = os.path.basename(storage_path)
-        renamed = os.path.join(os.path.dirname(tmp_path), safe_filename)
+        renamed = os.path.join(os.path.dirname(tmp_path), file.filename)
         os.rename(tmp_path, renamed)
         await asyncio.to_thread(ingest_file, renamed, None, current_user.id)
         os.unlink(renamed)
         logger.info(f"Uploaded and ingested: {file.filename}")
         return JSONResponse(status_code=200, content={"message": f"Successfully uploaded and ingested {file.filename}"})
     except Exception as e:
         logger.error(f"Ingestion failed, cleaning up storage file: {e}")
-        await storage_client.remove_files(STORAGE_BUCKET, [storage_path], auth_token=current_user.token)
+        try:
+            await storage_client.remove_files(STORAGE_BUCKET, [storage_path], auth_token=current_user.token)
+        except Exception:
+            logger.error(f"Cleanup also failed for storage path: {storage_path}")
         return JSONResponse(status_code=500, content={"message": str(e)})
 
 
@@ -505,8 +506,7 @@ async def upload_temporal_document(thread_id: str, file: UploadFile = File(...),
         tmp_path = await _download_from_storage(storage_path, auth_token=current_user.token)
         if not tmp_path:
             return JSONResponse(status_code=500, content={"message": "Failed to retrieve uploaded file for ingestion"})
-        safe_filename = os.path.basename(storage_path)
-        renamed = os.path.join(os.path.dirname(tmp_path), safe_filename)
+        renamed = os.path.join(os.path.dirname(tmp_path), file.filename)
         os.rename(tmp_path, renamed)
         await asyncio.to_thread(ingest_file, renamed, thread_id, current_user.id)
         os.unlink(renamed)

codex-backend/src/storage_client.py

@@ -1,4 +1,5 @@
 import os
+import urllib.parse
 
 import httpx
 from dotenv import load_dotenv
@@ -24,8 +25,12 @@ def _auth_headers(auth_token: str | None = None) -> dict:
     }
 
 
+def _encode_path(path: str) -> str:
+    return urllib.parse.quote(path, safe="/")
+
+
 async def upload_file(bucket: str, path: str, data: bytes, content_type: str | None = None, auth_token: str | None = None) -> None:
-    url = f"{_SUPABASE_URL}/storage/v1/object/{bucket}/{path}"
+    url = f"{_SUPABASE_URL}/storage/v1/object/{bucket}/{_encode_path(path)}"
     headers = _auth_headers(auth_token)
     headers["Content-Type"] = content_type or "application/octet-stream"
     headers["x-upsert"] = "true"
@@ -34,18 +39,24 @@ async def upload_file(bucket: str, path: str, data: bytes, content_type: str | N
             resp = await client.post(url, headers=headers, content=data)
             resp.raise_for_status()
             logger.info(f"Uploaded to storage: {bucket}/{path}")
+        except httpx.HTTPStatusError:
+            logger.error(f"Storage upload failed for {bucket}/{path}: status={resp.status_code} body={resp.text}")
+            raise
         except Exception as e:
             logger.error(f"Storage upload failed for {bucket}/{path}: {e}")
             raise
 
 
 async def download_file(bucket: str, path: str, auth_token: str | None = None) -> bytes:
-    url = f"{_SUPABASE_URL}/storage/v1/object/{bucket}/{path}"
+    url = f"{_SUPABASE_URL}/storage/v1/object/{bucket}/{_encode_path(path)}"
     async with httpx.AsyncClient(timeout=30) as client:
         try:
             resp = await client.get(url, headers=_auth_headers(auth_token))
             resp.raise_for_status()
             return resp.content
+        except httpx.HTTPStatusError:
+            logger.error(f"Storage download failed for {bucket}/{path}: status={resp.status_code} body={resp.text}")
+            raise
         except Exception as e:
             logger.error(f"Storage download failed for {bucket}/{path}: {e}")
             raise
@@ -58,9 +69,12 @@ async def list_files(bucket: str, prefix: str, auth_token: str | None = None) ->
             resp = await client.post(url, headers=_auth_headers(auth_token), json={"prefix": prefix, "limit": 100, "offset": 0})
             resp.raise_for_status()
             return resp.json()
+        except httpx.HTTPStatusError:
+            logger.error(f"Storage list failed for {bucket}/{prefix}: status={resp.status_code} body={resp.text}")
+            raise
         except Exception as e:
             logger.error(f"Storage list failed for {bucket}/{prefix}: {e}")
-            return []
+            raise
 
 
 async def remove_files(bucket: str, paths: list[str], auth_token: str | None = None) -> None:
@@ -69,8 +83,12 @@ async def remove_files(bucket: str, paths: list[str], auth_token: str | None = N
         try:
             resp = await client.post(url, headers=_auth_headers(auth_token), json={"prefixes": paths})
             resp.raise_for_status()
+        except httpx.HTTPStatusError:
+            logger.error(f"Storage remove failed for {bucket}/{paths}: status={resp.status_code} body={resp.text}")
+            raise
         except Exception as e:
             logger.error(f"Storage remove failed for {bucket}/{paths}: {e}")
+            raise
 
 
 async def ensure_bucket(bucket: str) -> None: