Skip to content

Commit 44911fa

Browse files
committed
fix(acp): give each replayed turn its own boundary
Reloading a session ran consecutive prompts together: run asyncio.sleep(60)run asyncio.sleep(60)One more time running... update_user_message emits a *chunk*, and ACP has no end-of-message marker — a boundary is implied by a different update type arriving. So two turns from the same speaker in a row land in one bubble. That happens whenever a turn produced no reply, which is exactly what a cancelled turn used to do, so a run of stopped prompts replayed as a single run-on line. Replay now terminates each turn. Cancelled turns also record "Stopped at your request." now, so new sessions have an agent turn between prompts as well, but the boundary belongs in replay regardless: any two same-role turns in a row would otherwise merge, and existing sessions still hold runs of unanswered prompts. Found by testing a real Zed reload. Signed-off-by: Paul Furgale <pfurgale@nvidia.com>
1 parent da64334 commit 44911fa

2 files changed

Lines changed: 67 additions & 3 deletions

File tree

packages/nooa-acp/src/nooa_acp/server.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,14 @@ async def _replay_session(self, handle: SessionHandle) -> None:
512512
if self._client is None:
513513
return
514514
for turn in handle.turns():
515-
block = text_block(turn.content)
515+
# Each update is a *chunk*, and ACP has no end-of-message marker —
516+
# a boundary is implied by a different update type arriving. Two
517+
# turns from the same speaker in a row therefore land in one bubble.
518+
# That happens whenever a turn produced no reply, as a cancelled one
519+
# used to, so several stopped prompts replayed as a single run-on
520+
# line. Terminate each turn so it keeps its own boundary.
521+
content = turn.content if turn.content.endswith("\n") else turn.content + "\n"
522+
block = text_block(content)
516523
update = (
517524
update_user_message(block) if turn.role == "user" else update_agent_message(block)
518525
)

packages/nooa-acp/tests/test_server.py

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,13 +1080,15 @@ async def test_adapter_lists_closes_loads_and_replays_durable_session(tmp_path):
10801080
for update in replay_client.updates
10811081
if isinstance(update, AgentMessageChunk)
10821082
]
1083-
assert replayed_text == ["ACP response"]
1083+
# Replay terminates each turn so consecutive turns from one speaker keep
1084+
# their own boundary; see test_replay_separates_consecutive_turns_from_one_speaker.
1085+
assert replayed_text == ["ACP response\n"]
10841086
replayed_user_text = [
10851087
update.content.text
10861088
for update in replay_client.updates
10871089
if isinstance(update, UserMessageChunk)
10881090
]
1089-
assert replayed_user_text == ["remember this"]
1091+
assert replayed_user_text == ["remember this\n"]
10901092
await replay_adapter.close()
10911093

10921094

@@ -1314,3 +1316,58 @@ async def test_mcp_server_with_a_reserved_name_does_not_kill_the_session(tmp_pat
13141316
session = await _session(adapter, created.session_id)
13151317
assert any("runtime" in warning for warning in session.startup_warnings)
13161318
await adapter.close()
1319+
1320+
1321+
async def test_a_skipped_mcp_server_is_reported_to_the_client(tmp_path):
1322+
"""The startup warning must reach the user, not just session state.
1323+
1324+
A server whose name collides is skipped so the session stays usable, but
1325+
silently skipping it means the user configured a server and gets no tools
1326+
and no explanation.
1327+
"""
1328+
client = _RecordingClient()
1329+
adapter = CodingACPAdapter(_completed_llm)
1330+
adapter.on_connect(client) # type: ignore[arg-type]
1331+
server = McpServerStdio(name="shell", command="lookup-server", args=[], env=[])
1332+
1333+
with patch(
1334+
"nooa_acp.server.MCPManager.create_stdio_server",
1335+
new=AsyncMock(return_value=_MCPTools()),
1336+
):
1337+
await adapter.new_session(str(tmp_path), mcp_servers=[server])
1338+
1339+
await asyncio.sleep(0.1) # let the deferred bootstrap publish
1340+
rendered = "".join(str(update) for update in client.updates)
1341+
assert "shell" in rendered, rendered[:400]
1342+
await adapter.close()
1343+
1344+
1345+
async def test_replay_separates_consecutive_turns_from_one_speaker(tmp_path):
1346+
"""Consecutive same-role turns must not run together on reload.
1347+
1348+
update_user_message emits a *chunk*, and ACP has no end-of-message marker:
1349+
a boundary is implied by a different update type arriving. Cancelled turns
1350+
recorded the prompt and produced nothing else, so several in a row replayed
1351+
into one bubble — "run sleep(60)run sleep(60)One more time...".
1352+
"""
1353+
client = _RecordingClient()
1354+
adapter = CodingACPAdapter(_completed_llm)
1355+
adapter.on_connect(client) # type: ignore[arg-type]
1356+
1357+
created = await adapter.new_session(str(tmp_path))
1358+
session = await _session(adapter, created.session_id)
1359+
session.handle.record_user_message("first")
1360+
session.handle.record_user_message("second")
1361+
await adapter.close_session(created.session_id)
1362+
1363+
client.updates.clear()
1364+
await adapter.load_session(str(tmp_path), created.session_id)
1365+
1366+
replayed = [
1367+
update.content.text
1368+
for update in client.updates
1369+
if type(update).__name__ == "UserMessageChunk"
1370+
]
1371+
assert len(replayed) == 2, replayed
1372+
assert "firstsecond" not in "".join(replayed)
1373+
await adapter.close()

0 commit comments

Comments
 (0)