diff --git a/backend/src/backend/scripts.py b/backend/src/backend/scripts.py index 8b18722..a48f94f 100644 --- a/backend/src/backend/scripts.py +++ b/backend/src/backend/scripts.py @@ -1196,13 +1196,34 @@ async def update_script( ) from exc script.updated_at = datetime.now(UTC).replace(tzinfo=None) - storage_data = { - "storage_object_id": script.current_object_id, - "relative_path": jupyter_path, - "object_key": f"{workspace_id}/{jupyter_path}", - "content_hash": hashlib.sha256(content).hexdigest(), - "size_bytes": len(content), - } + content_hash = hashlib.sha256(content).hexdigest() + size_bytes = len(content) + relative_path = f"workspace/{jupyter_path}" + + # Persist the new content fingerprint so cache/dedup/hash checks + # downstream see the post-edit values. Filename is fixed in + # update_script, so object_key / path_hash are unchanged — only the + # body-derived fields move. relative_path is re-derived to keep the + # response in sync with the workspace-prefixed convention used at + # create time. + if storage_object is not None: + storage_object.content_hash = content_hash + storage_object.size_bytes = size_bytes + storage_object.relative_path = relative_path + await session.flush() + await session.refresh(storage_object) + storage_data: StorageObjects | dict[str, Any] = storage_object + else: + # Jupyter-only script: no StorageObjects row to update, but the + # response still needs the workspace-prefixed path shape so the + # frontend's slice(2) reducer produces the expected basename. + storage_data = { + "storage_object_id": script.current_object_id, + "relative_path": relative_path, + "object_key": f"{workspace_id}/{jupyter_path}", + "content_hash": content_hash, + "size_bytes": size_bytes, + } return { "request_id": context.request_id, "data": script_payload(script, storage_data), diff --git a/backend/tests/test_scripts.py b/backend/tests/test_scripts.py index d8caade..cc97c79 100644 --- a/backend/tests/test_scripts.py +++ b/backend/tests/test_scripts.py @@ -343,6 +343,8 @@ def test_scripts_model_allows_duplicate_current_object_id() -> None: def _script_row() -> Scripts: + from datetime import UTC, datetime + return Scripts( script_id="01SCR0000000000000000000A", workspace_id="01WS0000000000000000000A", @@ -352,6 +354,8 @@ def _script_row() -> Scripts: script_type="notebook", visibility="private", status="active", + created_at=datetime.now(UTC).replace(tzinfo=None), + updated_at=datetime.now(UTC).replace(tzinfo=None), ) @@ -516,3 +520,147 @@ async def test_jupyter_check_notebook_lock_ignores_deleted_scripts() -> None: statement = call[0][0] compiled = str(statement.compile(compile_kwargs={"literal_binds": True})) assert "is_deleted" in compiled + + +@pytest.mark.asyncio +async def test_update_script_writes_back_storage_object_metadata( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """update_script must persist the new content_hash/size_bytes/relative_path + onto the StorageObjects row (not just into the response dict). Pre-fix the + DB row kept the create-time values forever, so cache/dedup/hash checks + downstream saw stale data. + + Also locks in the workspace-prefixed relative_path convention so the + frontend's slice(2) reducer produces a non-empty path even for root scripts. + """ + import hashlib + + from backend.schemas import UpdateScriptRequest + from backend.scripts import update_script + + script = _script_row() + storage_object = _storage_object_row() + # Seed stale metadata as it would have looked after create_script_record. + user_id = "01USR0000000000000000000A" + storage_object.object_key = f"{script.workspace_id}/{user_id}/test.ipynb" + storage_object.content_hash = hashlib.sha256(b"old").hexdigest() + storage_object.size_bytes = 3 + storage_object.relative_path = f"workspace/{user_id}/test.ipynb" + + request = _make_request() + runtime_client = request.app.state.runtime_client + runtime_client.upload_file = AsyncMock(return_value={"name": "test.ipynb"}) + runtime_client.create_notebook = AsyncMock(return_value={"name": "test.ipynb"}) + context = _make_context() + session = AsyncMock() + session.flush = AsyncMock() + session.refresh = AsyncMock() + + async def _fake_get_script_row( + _script_id: str, + _context: SimpleNamespace, + _session: AsyncMock, + *, + for_update: bool = False, + allow_missing_storage_object: bool = False, + ) -> tuple[Scripts, StorageObjects]: + return script, storage_object + + monkeypatch.setattr("backend.scripts.get_script_row", _fake_get_script_row) + + new_content = '{"cells": [{"cell_type": "code", "source": ["print(1)"]}]}\n' + payload = UpdateScriptRequest(content=new_content) + + result = await update_script( + script_id=script.script_id, + payload=payload, + request=request, + context=context, + session=session, + ) + + expected_hash = hashlib.sha256(new_content.encode("utf-8")).hexdigest() + expected_size = len(new_content.encode("utf-8")) + + # DB row carries the new metadata. + assert storage_object.content_hash == expected_hash + assert storage_object.size_bytes == expected_size + assert storage_object.relative_path == f"workspace/{user_id}/test.ipynb" + + # Response echoes the same values. + data = result["data"] + assert data["content_hash"] == expected_hash + assert data["size_bytes"] == expected_size + assert data["relative_path"] == f"workspace/{user_id}/test.ipynb" + + # session.flush + refresh were called to push the new metadata to DB. + session.flush.assert_awaited_once() + session.refresh.assert_awaited_once_with(storage_object) + # The runtime client received the create_notebook for the Jupyter path + # (script_type='notebook' goes through create_notebook, not upload_file). + runtime_client.create_notebook.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_update_script_jupyter_only_uses_dict_fallback( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When a script has no StorageObjects row (jupyter-only fallback path), + update_script must still return the workspace-prefixed relative_path + so the frontend's slice(2) reducer yields the basename — not an empty + string. No DB write should be attempted when there is no row to update. + """ + import hashlib + + from backend.schemas import UpdateScriptRequest + from backend.scripts import update_script + + script = _script_row() + user_id = "01USR0000000000000000000A" + + request = _make_request() + runtime_client = request.app.state.runtime_client + runtime_client.upload_file = AsyncMock(return_value={"name": "test.ipynb"}) + runtime_client.create_notebook = AsyncMock(return_value={"name": "test.ipynb"}) + context = _make_context() + session = AsyncMock() + session.flush = AsyncMock() + session.refresh = AsyncMock() + + async def _fake_get_script_row( + _script_id: str, + _context: SimpleNamespace, + _session: AsyncMock, + *, + for_update: bool = False, + allow_missing_storage_object: bool = False, + ) -> tuple[Scripts, None]: + return script, None + + monkeypatch.setattr("backend.scripts.get_script_row", _fake_get_script_row) + + payload = UpdateScriptRequest(content='{"cells": []}\n') + result = await update_script( + script_id=script.script_id, + payload=payload, + request=request, + context=context, + session=session, + ) + + expected_hash = hashlib.sha256(b'{"cells": []}\n').hexdigest() + + data = result["data"] + # Jupyter-only fallback derives jupyter_path from the script_id via + # _jupyter_path(), which appends ".ipynb". The contract we lock in is + # "always workspace-prefixed, never bare user_id/basename" so the + # frontend's slice(2) reducer is safe. + assert data["relative_path"] == f"workspace/{script.script_id}.ipynb" + assert data["relative_path"].startswith("workspace/") + assert data["content_hash"] == expected_hash + assert data["size_bytes"] == len(b'{"cells": []}\n') + + # No DB write should be attempted when there is no StorageObjects row. + session.flush.assert_not_awaited() + session.refresh.assert_not_awaited()