fix(tui): stop a stream outlived by /new from leaking into the next conversation
package / wheel (pull_request) Waiting to run

_finish_stream appended the completed reply to self.history - whatever
list that name currently pointed at - not to the conversation the
stream was actually answering. /new reassigns self.history to a fresh
list; a stream still running when that happens finished by silently
appending the old conversation's trailing reply onto the new one, which
then rides along in that new conversation's next /chat/stream history
payload. The conversation_id was already captured by closure for this
exact reason (see the tool-denial path); self.history needed the same
treatment.
This commit is contained in:
Jon Wingender
2026-08-26 14:01:19 -05:00
23 changed files with 1803 additions and 125 deletions
-2
View File
@@ -55,12 +55,10 @@ def test_legacy_cli_spellings_remain_compatible():
assert _normalize_legacy_argv(["start", "-b"]) == ["start", "backend"]
assert _normalize_legacy_argv(["stop", "--ai"]) == ["stop", "ai"]
assert _normalize_legacy_argv(["logs", "-m", "--follow"]) == ["logs", "memory", "--follow"]
assert _normalize_legacy_argv(["backup", "full"]) == ["backup", "--full"]
# -f is --follow for `logs`, but --frontend for start/stop. Translating it
# for logs turned `logs -f` into a one-shot tail of the frontend log.
assert _normalize_legacy_argv(["logs", "-f"]) == ["logs", "-f"]
assert _normalize_legacy_argv(["logs", "-m", "-f"]) == ["logs", "memory", "-f"]
assert _normalize_legacy_argv(["start", "-f"]) == ["start", "frontend"]
assert _normalize_legacy_argv(["restore", "-f"]) == ["restore"]
assert _normalize_legacy_argv(["help"]) == ["--help"]
+1 -2
View File
@@ -22,7 +22,6 @@ def test_render_frame_contains_sections():
"version": "0.0.0",
"services": {
"backend": {"running": True, "pid": 11, "url": "http://127.0.0.1:8000"},
"memory": {"running": False, "pid": None, "url": "http://127.0.0.1:8001"},
"frontend": {"running": False, "pid": None, "url": "http://127.0.0.1:5173"},
"provider": {
"provider": "ollama",
@@ -55,7 +54,7 @@ def test_render_frame_contains_sections():
assert "DATA / TOOLS" in frame
assert "RUN TOOLCHAINS" in frame
assert "backend" in frame and "UP" in frame
assert "memory" in frame and "DOWN" in frame
assert "frontend" in frame and "DOWN" in frame
assert "run_snippet" in frame
assert "ready python" in frame
assert "missing rust" in frame
+14
View File
@@ -546,6 +546,20 @@ def test_update_apply_spawns_detached_and_refuses_a_second_run(monkeypatch):
assert client.post("/update/apply").json()["started"] is False
def test_preview_iframe_cannot_navigate_to_a_network_url():
"""The child CSP blocks resource loads; the parent CSP must separately
block a sandboxed frame from navigating its own browsing context."""
index = (REPO_ROOT / "interface" / "web" / "index.html").read_text(encoding="utf-8")
markdown = (REPO_ROOT / "interface" / "web" / "src" / "Markdown.jsx").read_text(
encoding="utf-8"
)
assert "frame-src data:" in index
assert 'sandbox="allow-scripts"' in markdown
assert "encodeURIComponent(doc)" in markdown
assert "src={frameUrl}" in markdown
assert "srcDoc={doc}" not in markdown
def test_ollama_failures_surface_the_reason_not_just_the_status():
"""Ollama answers every failure with {"error": "..."} and httpx's default
message throws it away. A user hitting a retired cloud model saw
+432 -4
View File
@@ -7,6 +7,7 @@ Guards the two pieces that would silently break the feature: the allowlist
filter and the tool-call loop's terminate-on-content behaviour.
"""
import asyncio
import json
from synapse import tools
from synapse.chat import _run_tool_loop
@@ -63,7 +64,8 @@ def _drive_with_decision(decision, monkeypatch):
async def run():
messages = [{"role": "user", "content": "remember x"}]
gen = chatmod._run_tool_loop(_ActionManager(), messages, "m", [{}], None, None,
schemas = tools.schemas_for(["remember"])
gen = chatmod._run_tool_loop(_ActionManager(), messages, "m", schemas, None, None,
conversation_id="conv", policy="ask")
statuses = []
async for s in gen:
@@ -91,6 +93,52 @@ def test_ask_policy_skips_on_deny(monkeypatch):
assert any(m["role"] == "tool" and "declined" in m["content"] for m in messages)
class _ContentJsonActionManager:
"""Small-model shape: dumps the action call into `content`, no native
`tool_calls` field — the lower-confidence path the "allow" bypass must
not trust."""
def __init__(self):
self.n = 0
async def chat(self, **_):
self.n += 1
if self.n == 1:
return {"role": "assistant",
"content": json.dumps({"name": "remember", "arguments": {"text": "x"}})}
return {"role": "assistant", "content": "done"}
def test_content_json_action_call_asks_even_under_allow_policy(monkeypatch):
"""A call recovered by guessing at `content` is weaker evidence than the
API's own structured tool_calls field — a model can land on JSON shaped
like a call while only meaning to describe one. It must still go through
approval even when action_tool_policy is "allow", the default that lets a
*native* tool_calls field run unattended."""
from synapse import chat as chatmod
async def fake_dispatch(name, args):
return "saved-ok"
monkeypatch.setattr(tools, "dispatch", fake_dispatch)
async def run():
messages = [{"role": "user", "content": "remember x"}]
schemas = tools.schemas_for(["remember"])
gen = chatmod._run_tool_loop(_ContentJsonActionManager(), messages, "m", schemas, None, None,
conversation_id="conv", policy="allow")
statuses = []
async for s in gen:
statuses.append(s)
if s.startswith("__approve__"):
w = chatmod.pending_approvals["conv"]
w["decisions"] = {"remember": True}
w["event"].set()
return statuses
statuses = asyncio.run(run())
assert any(s.startswith("__approve__") for s in statuses)
assert "__status__remember" in statuses
def test_action_tools_gated_by_consent():
allow = ["search_memory", "web_search", "remember", "fetch_url"]
on = [s["function"]["name"] for s in tools.schemas_for(allow, allow_actions=True)]
@@ -131,8 +179,8 @@ def test_tool_loop_runs_tool_then_stops(monkeypatch):
_run_tool_loop(_FakeManager(), messages, "m", schemas, None, None)
))
# one status sentinel per tool run
assert statuses == ["__status__search_memory"]
# heartbeat + one status sentinel per tool run
assert statuses == ["__status__tools", "__status__search_memory"]
# messages mutated in place: user -> assistant(tool_calls) -> tool(result);
# the final content turn is NOT appended (the streaming turn regenerates it).
assert [m["role"] for m in messages] == ["user", "assistant", "tool"]
@@ -147,7 +195,7 @@ def test_tool_loop_degrades_when_model_returns_no_dict():
messages = [{"role": "user", "content": "hi"}]
before = list(messages)
statuses = asyncio.run(_drain(_run_tool_loop(_NoToolManager(), messages, "m", [{}], None, None)))
assert statuses == [] # no tool ran
assert statuses == ["__status__tools"] # heartbeat only; no tool ran
assert messages == before # untouched -> falls back to a plain stream
@@ -206,3 +254,383 @@ def test_routed_reference_playbook_contributes_its_tools(tmp_path, monkeypatch):
assert {"read_file", "list_files"} <= granted, granted
# none of them are action tools, so they survive the default policy (off)
assert tools.schemas_for(sorted(granted), allow_actions=False)
def test_standing_schemas_include_render_preview():
names = [s["function"]["name"] for s in tools.standing_schemas()]
assert names == ["render_preview"]
assert "render_preview" in tools.STANDING_TOOLS
assert not tools.is_action("render_preview")
assert tools.wants_render_preview("visualize Collatz with a chart")
assert not tools.wants_render_preview("what's the weather vibe today")
def test_render_preview_packages_markup_without_grading_its_quality():
markup = """<!DOCTYPE html><html><body>
<canvas id="c" width="40" height="40"></canvas>
<script>c.width = c.width;</script>
</body></html>"""
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "html", "title": "Demo", "markup": markup,
})))
assert out["ok"] is True
assert markup in out["fence"]
assert "issues" not in out
assert "scaffold" not in out
def test_render_preview_accepts_canvas_that_plots():
good = """<!DOCTYPE html><html><body>
<canvas id="c" width="480" height="240"></canvas>
<input id="n" type="number" value="27">
<button onclick="go()">Go</button>
<script>
const c = document.getElementById('c');
const ctx = c.getContext('2d');
function go() {
let n = +document.getElementById('n').value, seq = [];
while (n !== 1 && seq.length < 500) { seq.push(n); n = n % 2 === 0 ? n/2 : 3*n+1; }
seq.push(1);
const max = Math.max(...seq);
ctx.clearRect(0,0,c.width,c.height);
ctx.beginPath();
seq.forEach((v,i) => {
const x = i * (c.width / Math.max(1, seq.length-1));
const y = c.height - (v / max) * (c.height - 8);
if (i === 0) ctx.moveTo(x,y); else ctx.lineTo(x,y);
});
ctx.stroke();
}
go();
</script></body></html>"""
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "html", "markup": good, "purpose": "line plot of an iterative sequence",
})))
assert out["ok"] is True
assert out["fence"].startswith("```html\n")
assert "getContext" in out["fence"]
assert out.get("repaired") is not True
def test_code_that_throws_is_left_to_the_previews_own_error_channel():
"""This markup is broken twice over: getContext() is assigned to `canvas`
but drawn with `ctx`, and collatz() is called as coll(). Both used to be
rejected here by regex. Both now reach the browser, which reports them
precisely — verified against the real preview:
"Uncaught ReferenceError: ctx is not defined (line 4)"
"Uncaught ReferenceError: coll is not defined (line 5)"
Static guessing at runtime failures only ever caught the spellings someone
anticipated; the error channel catches every one of them and carries a line
number."""
broken_at_runtime = """<!DOCTYPE html><html><body>
<canvas id="c" width="480" height="280"></canvas>
<script>
const canvas = document.getElementById('c').getContext('2d');
function collatz(n) {
const s = [];
while (n !== 1 && s.length < 500) {
s.push(n);
n = n % 2 === 0 ? n / 2 : n * 3 + 1;
}
s.push(1);
return s;
}
function plot() {
const seq = coll(document.getElementById('n').value);
const max = Math.max(...seq), w = canvas.width, h = canvas.height;
ctx.clearRect(0, 0, w, h);
ctx.beginPath();
seq.forEach((v, i) => {
const x = i * (w / Math.max(1, seq.length - 1));
const y = h - (v / max) * h;
if (i === 0) ctx.moveTo(x, y); else ctx.lineTo(x, y);
});
ctx.stroke();
}
</script></body></html>"""
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "html",
"purpose": "interactive sequence plot",
"markup": broken_at_runtime,
})))
assert out["ok"] is True, out.get("issues")
def test_no_sequence_render_seed_helper():
assert not hasattr(tools, "sequence_render_seed")
_FRONTEND_REGISTRY = ("interface", "web", "src", "preview", "languages.js")
def _frontend_preview_langs() -> list[str]:
"""Top-level keys of PREVIEW_LANGS in the frontend's preview registry."""
import re
from pathlib import Path
src = Path(__file__).resolve().parents[1].joinpath(*_FRONTEND_REGISTRY)
text = src.read_text(encoding="utf-8")
body = re.search(r"^export const PREVIEW_LANGS = \{\n(.*?)^\};", text, re.S | re.M)
assert body, f"could not find a PREVIEW_LANGS object literal in {src}"
return re.findall(r"^ (\w+):", body.group(1), re.M)
def test_preview_langs_match_the_frontend_registry():
"""The render window is two registries — synapse/tools.py validates a
language, interface/web/src/Markdown.jsx renders it — and a language present
in only one degrades silently: the model emits a fence the UI shows as a
plain code block, or the UI offers a preview the tool refuses to produce.
Nothing at runtime couples them, so this is what keeps them in step."""
# Plain ASCII in the message: this is read off a Windows console, where
# pytest's output encoding mangles non-ASCII into replacement characters.
assert _frontend_preview_langs() == list(tools.PREVIEW_LANGS), (
"PREVIEW_LANGS differs between synapse/tools.py and "
"interface/web/src/Markdown.jsx - add the language to both."
)
def test_preview_lang_enum_is_derived_not_repeated():
schema, _ = tools.REGISTRY["render_preview"]
enum = schema["function"]["parameters"]["properties"]["lang"]["enum"]
assert enum == list(tools.PREVIEW_LANGS)
def test_render_preview_rejects_unknown_lang():
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "python", "markup": "print('hi')" * 5,
})))
assert out["ok"] is False
assert "lang must be" in out["error"]
def test_render_preview_accepts_a_jsx_component():
good = """export default function Counter() {
const [n, setN] = useState(0);
return (
<div>
<button onClick={() => setN(n + 1)}>count {n}</button>
</div>
);
}"""
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "jsx", "markup": good, "purpose": "interactive counter",
})))
assert out["ok"] is True, out.get("issues")
assert out["fence"].startswith("```jsx\n")
def test_render_preview_does_not_grade_jsx_against_its_purpose():
component = """export default function Form() {
const [name, setName] = useState("");
return <label>Name <input value={name} onInput={(e) => setName(e.target.value)} /></label>;
}"""
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "jsx", "markup": component, "purpose": "a chart of the results",
})))
assert out["ok"] is True
assert "issues" not in out
def test_asking_for_a_preview_language_or_pointer_interaction_offers_the_tool():
"""Each of these is a real prompt from a transcript where the render window
should have been reachable. The first one was not: no hint matched
'mouse-over sensitive ... jsx', so the tool was never advertised and the
model answered about Euler's formula instead."""
for prompt in (
"Create a mouse-over sensitive Euler fluid field as a jsx or tsx",
"write me a small tsx component",
"make the particles react to hover",
"a real-time simulation I can drag",
):
assert tools.wants_render_preview(prompt), prompt
# Still narrow: ordinary chat must not pay for a tool turn.
for prompt in (
"what's the weather vibe today",
"summarise this email thread",
"write a concise paragraph about caching",
):
assert not tools.wants_render_preview(prompt), prompt
assert tools.wants_render_preview("compare these graphs")
def test_external_preview_resources_are_packaged_for_the_csp_to_block():
markup = '<img src="https://example.com/chart.png" alt="chart">'
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "html", "markup": markup,
})))
assert out["ok"] is True
assert markup in out["fence"]
assert "issues" not in out
def test_every_preview_language_hints_for_itself():
for lang in tools.PREVIEW_LANGS:
assert lang in tools._RENDER_HINTS, lang
def test_normal_tool_results_reach_streaming_turn():
"""Flatten Ollama's tool roles without discarding the retrieved data."""
from synapse.chat import _strip_internal_turns
request = {"role": "user", "content": "what GPU do I have?"}
kept = _strip_internal_turns([
request,
{"role": "assistant", "content": "", "tool_calls": [{
"function": {"name": "search_memory", "arguments": {"query": "GPU"}},
}]},
{"role": "tool", "content": '[{"text":"Vega 20 4GB"}]'},
])
assert kept[-1] == request
assert "Vega 20 4GB" in kept[-2]["content"]
assert all(m.get("role") != "tool" and not m.get("tool_calls") for m in kept)
def test_render_preview_leaves_jsx_runtime_judgment_to_the_browser():
sources = (
"const x = 1;\nconsole.log(x);\n// nothing to mount",
'import { motion } from "framer-motion"; export default () => <motion.div />;',
"export default () => <div style={{width: 40}}>tiny</div>;",
)
for source in sources:
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "jsx", "markup": source,
})))
assert out["ok"] is True
assert source in out["fence"]
assert "issues" not in out
def test_render_preview_allows_react_imports_in_jsx():
src = """import { useState } from "react";
export default function App() {
const [n] = useState(0);
return <p>count is {n} right now</p>;
}"""
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "jsx", "markup": src,
})))
assert out["ok"] is True, out.get("issues")
def test_render_preview_still_rejects_missing_markup():
out = json.loads(asyncio.run(tools.dispatch("render_preview", {
"lang": "tsx", "markup": "",
})))
assert out["ok"] is False
assert "markup is required" in out["error"]
assert "scaffold" not in out
def test_coerce_tool_calls_from_content_json():
from synapse.chat import _coerce_tool_calls
# Structured field wins.
structured = {"role": "assistant", "tool_calls": [
{"function": {"name": "get_time", "arguments": {}}}
]}
assert _coerce_tool_calls(structured)[0]["function"]["name"] == "get_time"
# Small models dump a complete call into content.
content_call = {
"role": "assistant",
"content": '{"name":"render_preview","arguments":{"lang":"svg","markup":"<svg/>"}}',
}
calls = _coerce_tool_calls(content_call, {"render_preview"})
assert len(calls) == 1
assert calls[0]["function"]["name"] == "render_preview"
assert calls[0]["function"]["arguments"]["lang"] == "svg"
# JSON quoted as part of an explanation is output, not an instruction to
# execute a tool (especially important for action tools such as remember).
embedded = {
"role": "assistant",
"content": (
'For example: {"name":"remember","arguments":{"text":"do not save"}} '
"is the tool-call shape."
),
}
assert _coerce_tool_calls(embedded, {"remember"}) == []
# Even a whole JSON object cannot call a tool that was not advertised.
assert _coerce_tool_calls(content_call, {"search_memory"}) == []
def test_tool_loop_runs_content_json_tool_call(monkeypatch):
"""qwen-style: first turn returns content-JSON tool call, second returns text."""
from synapse import chat as chatmod
class _ContentJsonManager:
def __init__(self):
self.n = 0
async def chat(self, **_):
self.n += 1
if self.n == 1:
return {
"role": "assistant",
"content": json.dumps({
"name": "render_preview",
"arguments": {
"lang": "svg",
"markup": (
'<svg xmlns="http://www.w3.org/2000/svg" width="320" height="200">'
'<circle cx="160" cy="100" r="60" fill="red"/></svg>'
),
},
}),
}
return {"role": "assistant", "content": "done"}
statuses, messages = asyncio.run(_drain_with_messages(
_ContentJsonManager(), "m", tools.standing_schemas(),
user="draw a circle",
))
assert any(s == "__status__render_preview" for s in statuses)
tool_msgs = [m for m in messages if m.get("role") == "tool"]
assert tool_msgs
assert json.loads(tool_msgs[0]["content"])["ok"] is True
def test_tool_loop_does_not_inject_a_render_preview_nudge():
class _SkipThenCall:
def __init__(self):
self.n = 0
async def chat(self, **_):
self.n += 1
if self.n == 1:
return {"role": "assistant", "content": "Sure, here is a chart in prose."}
if self.n == 2:
return {
"role": "assistant",
"tool_calls": [{
"function": {
"name": "render_preview",
"arguments": {
"lang": "svg",
"markup": (
'<svg xmlns="http://www.w3.org/2000/svg" width="480" height="280">'
'<rect width="480" height="280" fill="#111"/>'
'<text x="24" y="150" fill="#eee" font-size="24">hi</text></svg>'
),
},
}
}],
}
return {"role": "assistant", "content": "done"}
statuses, messages = asyncio.run(_drain_with_messages(
_SkipThenCall(), "m", tools.standing_schemas(),
user="Visualize the Collatz conjecture with an interactive chart",
))
assert statuses == ["__status__tools"]
assert len(messages) == 1
assert messages[0]["content"].startswith("Visualize")
async def _drain_with_messages(manager, model, schemas, user="draw a circle"):
messages = [{"role": "user", "content": user}]
statuses = await _drain(
_run_tool_loop(manager, messages, model, schemas, None, None)
)
return statuses, messages
+67 -2
View File
@@ -98,7 +98,7 @@ def test_finish_stream_markup_does_not_wedge_busy():
async def _run():
async with app.run_test():
app._busy = True
app._finish_stream("see [/] and arr[i]")
app._finish_stream("see [/] and arr[i]", app.history)
assert app._busy is False
assert app.history[-1]["content"] == "see [/] and arr[i]"
@@ -115,7 +115,7 @@ def test_stream_error_remains_visible_after_finish():
async with app.run_test():
app._busy = True
app._show_error("[red]Backend not reachable[/]")
app._finish_stream("")
app._finish_stream("", app.history)
log = app.query_one("#log")
assert any("Backend not reachable" in line.text for line in log.lines)
assert app._busy is False
@@ -225,6 +225,71 @@ def test_inflight_tool_denial_uses_original_conversation_id(monkeypatch):
asyncio.run(_run())
def test_new_mid_stream_does_not_leak_reply_into_next_conversation(monkeypatch):
"""A stream still in flight when /new resets self.history must keep
appending its reply to the conversation it was actually answering, not
whatever self.history now points at - otherwise the old reply's text
silently rides along in the next request's history payload."""
pytest.importorskip("textual")
import nexusos_cli.tui_app as tui_app
stream_started = threading.Event()
release_stream = threading.Event()
class _StreamResponse:
status_code = 200
async def __aenter__(self):
return self
async def __aexit__(self, *args):
return None
async def aiter_lines(self):
stream_started.set()
await asyncio.to_thread(release_stream.wait, 2)
yield 'data: "the old reply"'
yield ""
yield "event: done"
yield "data: {}"
class _StreamClient:
def __init__(self, **kwargs):
pass
async def __aenter__(self):
return self
async def __aexit__(self, *args):
return None
def stream(self, *args, **kwargs):
return _StreamResponse()
monkeypatch.setattr(tui_app.httpx, "AsyncClient", _StreamClient)
app = tui_app.NexusTUI.build_app(api_url="http://127.0.0.1:9")
async def _run():
async with app.run_test():
app._start_chat("first question")
assert await asyncio.to_thread(stream_started.wait, 2)
old_history = app.history
app._handle_slash("/new")
assert app.history is not old_history
release_stream.set()
for _ in range(200):
if not app._busy:
break
await asyncio.sleep(0.01)
assert app._busy is False
# The reply landed on the abandoned conversation's own list...
assert any(m["content"] == "the old reply" for m in old_history)
# ...never on the fresh one /new started.
assert app.history == []
asyncio.run(_run())
def test_interrupt_cancels_silent_stream_and_accepts_next_message(monkeypatch):
pytest.importorskip("textual")
import nexusos_cli.tui_app as tui_app