diff --git a/tests/test_console_routing_proxy.py b/tests/test_console_routing_proxy.py index b6b21691..417b4f5b 100644 --- a/tests/test_console_routing_proxy.py +++ b/tests/test_console_routing_proxy.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from typing import Any from unittest.mock import MagicMock @@ -286,6 +287,67 @@ class TestRouteCreate503Retry: client.close() +# --------------------------------------------------------------------------- +# Tests — cluster create (capacity-routed proxy) +# --------------------------------------------------------------------------- + + +class TestClusterCreate: + """POST /v1/api/cluster/workstreams/new — the launcher's create proxy. + + Create-with-attachments rides multipart (a ``meta`` JSON field + ``file`` + parts) and must forward to the node AS multipart — not collapse to JSON, + which would silently drop the files (the pre-fix behaviour, gated in the UI + as "Attachments aren't supported for interactive sessions yet").""" + + def _app_with_node(self, mock_post: MagicMock) -> Any: + collector = _make_mock_collector() + collector.get_node_detail.return_value = {"server_url": "http://a:8080"} + app = _make_app(collector=collector) + _wire_proxy(app, mock_post) + return app + + def test_cluster_create_json_forwards_json(self): + mock_post = _make_proxy_post(json_data={"ws_id": "abc123"}) + client = TestClient(self._app_with_node(mock_post), raise_server_exceptions=False) + resp = client.post( + "/v1/api/cluster/workstreams/new", + json={"node_id": "node-a", "name": "j", "initial_message": "hi"}, + headers=_TEST_AUTH_HEADERS, + ) + assert resp.status_code == 200 + assert resp.json()["correlation_id"] == "abc123" + kwargs = mock_post.call_args.kwargs + assert "json" in kwargs and "files" not in kwargs, "no-file create must stay JSON" + assert kwargs["json"]["initial_message"] == "hi" + client.close() + + def test_cluster_create_multipart_forwards_files(self): + mock_post = _make_proxy_post(json_data={"ws_id": "withfile"}) + client = TestClient(self._app_with_node(mock_post), raise_server_exceptions=False) + meta = {"node_id": "node-a", "name": "i", "initial_message": "describe"} + resp = client.post( + "/v1/api/cluster/workstreams/new", + data={"meta": json.dumps(meta)}, + files={"file": ("a.txt", b"hello world", "text/plain")}, + headers=_TEST_AUTH_HEADERS, + ) + assert resp.status_code == 200 + assert resp.json()["correlation_id"] == "withfile" + kwargs = mock_post.call_args.kwargs + # Forwarded as multipart: a `meta` JSON field + `file` parts, never json=. + assert "json" not in kwargs, "a multipart create must not collapse to JSON" + assert kwargs.get("files"), "the blob must be forwarded to the node" + forwarded_meta = json.loads(kwargs["data"]["meta"]) + assert forwarded_meta["initial_message"] == "describe" + assert "user_id" in forwarded_meta, "the proxy must inject the owner uid" + # The file part carries our blob unchanged: ("file", (name, bytes, ctype)). + name, payload = kwargs["files"][0] + assert name == "file" + assert payload[0] == "a.txt" and payload[1] == b"hello world" + client.close() + + # --------------------------------------------------------------------------- # Tests — route_proxy # --------------------------------------------------------------------------- diff --git a/tests/test_shell_js.py b/tests/test_shell_js.py index 31fc3430..2446da63 100644 --- a/tests/test_shell_js.py +++ b/tests/test_shell_js.py @@ -291,6 +291,27 @@ def test_console_launcher_node_strategy() -> None: ) +def test_console_launcher_interactive_create_carries_attachments() -> None: + """Create-time attachments for interactive sessions: the launcher gate is gone + and ``_createInteractive`` frames a multipart body (``meta`` JSON + ``file`` + parts) when files are staged, so the cluster proxy can forward the blobs to + the node — mirroring ``_createCoordinator``. Was previously blocked with + "Attachments aren't supported for interactive sessions yet".""" + app = _CONSOLE_APP.read_text(encoding="utf-8") + assert "Attachments aren't supported for interactive sessions yet." not in app, ( + "the create-time interactive attachment gate must be removed" + ) + # Scope the multipart-framing assertions to _createInteractive's own body so + # they can't be satisfied by _createCoordinator alone. + rest = app[app.index("function _createInteractive(") + 1 :] + cut = rest.find("\nfunction ") + body = rest if cut == -1 else rest[:cut] + assert "new FormData()" in body, "_createInteractive must build a multipart body" + assert 'form.append("meta"' in body and 'form.append("file"' in body, ( + "_createInteractive must send meta JSON + file parts" + ) + + def test_pane_persists_meta_for_rehydrate() -> None: """Workstream-lifecycle bugfix: PaneManager persists a pane's serializable open-time meta (the interactive pane's resolved nodeId) and hands it back as diff --git a/turnstone/console/server.py b/turnstone/console/server.py index 12d3a735..1a0f8e0c 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -1766,8 +1766,9 @@ async def create_workstream(request: Request) -> JSONResponse: - ``node_id`` omitted or ``"auto"`` → console picks the node with most headroom - ``node_id`` set to ``"pool"`` → console picks any available node """ + from turnstone.core.attachments import IMAGE_SIZE_CAP from turnstone.core.auth import require_any_permission - from turnstone.core.web_helpers import read_json_or_400 + from turnstone.core.web_helpers import read_json_or_400, read_multipart_create_or_400 # Gate on workstreams.create OR admin.coordinator before proxying — # keeps the 403 attributed at the console (audit clarity) and avoids @@ -1779,9 +1780,30 @@ async def create_workstream(request: Request) -> JSONResponse: if err is not None: return err - body = await read_json_or_400(request) - if isinstance(body, JSONResponse): - return body + # Create-with-attachments: the launcher sends multipart (a ``meta`` JSON + # field + ``file`` parts) instead of JSON. The node create endpoint already + # accepts that shape (create_supports_attachments on interactive_endpoint_config + # in turnstone/server.py); the proxy just picks the node as usual and forwards + # the files instead of re-serialising JSON. Caps mirror the node-side parse so + # an oversized upload is rejected here, before the cluster hop. + content_type = (request.headers.get("content-type") or "").lower() + uploaded_files: list[tuple[str, str, bytes]] = [] + body: dict[str, Any] + if content_type.startswith("multipart/form-data"): + parsed = await read_multipart_create_or_400( + request, + max_files=10, + max_per_file_bytes=IMAGE_SIZE_CAP, + max_total_bytes=10 * IMAGE_SIZE_CAP, + ) + if isinstance(parsed, JSONResponse): + return parsed + body, uploaded_files = parsed + else: + json_body = await read_json_or_400(request) + if isinstance(json_body, JSONResponse): + return json_body + body = json_body collector: ClusterCollector = request.app.state.collector @@ -1865,12 +1887,22 @@ async def create_workstream(request: Request) -> JSONResponse: client: httpx.AsyncClient = request.app.state.proxy_client headers = _proxy_auth_headers(request) + node_url = f"{server_url.rstrip('/')}/v1/api/workstreams/new" try: - resp = await client.post( - f"{server_url.rstrip('/')}/v1/api/workstreams/new", - json=ws_body, - headers=headers, - ) + if uploaded_files: + # Re-frame for the node: create metadata rides one ``meta`` JSON + # field, each blob a ``file`` part — the shape the node's + # read_multipart_create_or_400 expects. ``user_id`` in the meta is + # honoured because the proxy auth header marks a ``console`` source. + files_payload = [("file", (fn, data, ctype)) for (fn, ctype, data) in uploaded_files] + resp = await client.post( + node_url, + data={"meta": json.dumps(ws_body)}, + files=files_payload, + headers=headers, + ) + else: + resp = await client.post(node_url, json=ws_body, headers=headers) resp.raise_for_status() except httpx.HTTPError as exc: log.warning("Workstream dispatch to %s failed: %s", node_id, exc) diff --git a/turnstone/console/static/app.js b/turnstone/console/static/app.js index 532a75a0..7b66b545 100644 --- a/turnstone/console/static/app.js +++ b/turnstone/console/static/app.js @@ -1173,11 +1173,28 @@ function _createInteractive(opts) { if (judgeModel) body.judge_model = judgeModel; if (task) body.initial_message = task; - authFetch("/v1/api/cluster/workstreams/new", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(body), - }) + // Multipart when files are staged — `meta` JSON + zero-or-more `file` + // parts. The cluster proxy picks the node (node_id in meta) and forwards + // the files to its create endpoint; plain JSON stays the default otherwise. + const files = Array.isArray(opts.files) ? opts.files : []; + let fetchOpts; + if (files.length > 0) { + const form = new FormData(); + form.append("meta", JSON.stringify(body)); + for (let i = 0; i < files.length; i++) { + form.append("file", files[i], files[i].name); + } + // Don't set Content-Type — the browser adds the correct boundary. + fetchOpts = { method: "POST", body: form }; + } else { + fetchOpts = { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }; + } + + authFetch("/v1/api/cluster/workstreams/new", fetchOpts) .then(function (r) { return r.json().then(function (data) { return { ok: r.ok, status: r.status, data: data }; @@ -1666,13 +1683,10 @@ function submitHomeCoord(textFromComposer) { }, }; if (kind === "interactive") { - // The cluster create proxy is JSON-only; attachments stay coordinator-only. - if (files.length > 0) { - _homeShowError( - "Attachments aren't supported for interactive sessions yet.", - ); - return; - } + // Create-with-attachments rides multipart through the cluster proxy to the + // node (see _createInteractive); the files-need-a-task guard above already + // ensures an initial turn to dispatch them on. + shared.files = files; // Node placement from the launcher's node-strategy picker (interactive-only). shared.node_strategy = opts.node_strategy || "auto"; shared.node_id = (opts.node_id || "").trim();