mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(attachments): forward create-time attachments for console interactive sessions
The console creates interactive sessions by proxying to the owning node via /v1/api/cluster/workstreams/new, which only forwarded JSON — so a file staged in the launcher was blocked with "Attachments aren't supported for interactive sessions yet". The node create endpoint already accepts multipart (meta JSON + file parts) on interactive_endpoint_config; only the proxy lacked it. Teach create_workstream to accept multipart: parse meta + files (same caps as the node), pick the node exactly as before (auto / pool / pinned), and forward the files instead of re-serialising JSON. _createInteractive sends multipart when files are staged (mirroring _createCoordinator) and the launcher gate is removed. The files-need-a-task guard already ensures an initial turn to dispatch them on.
This commit is contained in:
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user