feat(R04): immutable turn snapshot, typed agent events, conversation service

EPIC R04 (Team Duy) - the turn lifecycle leaves the widget.

R04-T01 domain/agents/conversation_execution_request.py
  Frozen snapshot of one turn, captured on the UI thread at submit time. The
  job closure used to read widget/workspace state from inside the worker
  thread, so a turn could run on a mix of submit-time and later state
  depending on thread timing.
R04-T02 domain/agents/agent_event.py
  13 frozen event types replacing untyped emit() dicts, with a two-way bridge
  so existing widgets keep consuming the legacy shape until EPIC R08. Adds
  TurnCompletedEvent - the end-of-turn signal the engine never had, which is
  why a cancelled turn and a failed turn look identical to the UI today.
R04-T03 application/conversations/conversation_application_service.py
  Runs a turn from a request and reports typed events. Never raises across the
  worker boundary; TurnResult.raise_if_failed() preserves the existing
  exception-based failure path. begin_turn()/execute_turn() expose the live
  message list for callers that autosave history mid-run.
R04-T04 ui/cowork_tab.py::build_job -> snapshot + service.
R04-T05 core/task_executors.py::_run_agent -> same service (was a second,
  slightly different assembly of the same call).

Caught while wiring the bridge: the first event vocabulary had no "notice"
event, so Agent Security warnings and auto-compaction notices would have been
silently swallowed. Added NoticeEvent plus a test that scans the engine sources
for emit() tags and fails when one has no typed counterpart.

New: tests/integration/ - real offscreen CoworkTab running a scripted turn end
to end (7 tests), including a characterisation of the extra provider call Agent
Security spends reviewing each request.

Suite: 225 passed, 2.74s. check_imports: PASS. All new files < 400 LOC.
2 pre-existing failures remain in test_config_security.py (EPIC R02/Team Nam).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-21 10:32:14 +09:00
co-authored by Claude Opus 5
parent 96bec976e7
commit a53163ebaf
11 changed files with 1663 additions and 53 deletions
+8
View File
@@ -0,0 +1,8 @@
"""Integration tests: real widgets, real services, no network (R10-T01 layout).
These build actual Qt widgets offscreen (``QT_QPA_PLATFORM=offscreen``) and run
a turn end to end with a scripted :class:`FakeProvider`. They are slower than
the unit suite - a QApplication has to exist - and are what proves the seams
introduced by R03/R04 are actually wired into the screens, not just correct in
isolation.
"""
+201
View File
@@ -0,0 +1,201 @@
"""End-to-end check that the Cowork screen really runs turns through the
application layer (R04-T04).
The unit tests prove ``ConversationApplicationService`` behaves correctly; this
one proves ``ui/cowork_tab.py::build_job`` actually goes through it, on a real
(offscreen) widget, with a scripted provider instead of a network call.
It also pins the property that motivated R04-T01: the turn runs on the state
captured at SUBMIT time, so a user editing the conversation while a turn is in
flight cannot change what that turn sends.
"""
from __future__ import annotations
import os
from pathlib import Path
from typing import Any, Dict, List
import pytest
os.environ.setdefault("QT_QPA_PLATFORM", "offscreen")
from cowork_local.config import AppConfig # noqa: E402
from cowork_local.core import chat_agent # noqa: E402
from cowork_local.state import AppContext # noqa: E402
from tests.fakes import FakeProvider, ScriptedTurn # noqa: E402
pytest.importorskip("PySide6", reason="Qt is required for the integration suite")
@pytest.fixture(scope="module")
def qt_app():
"""One QApplication for the module - Qt allows only a single instance."""
from PySide6.QtWidgets import QApplication
return QApplication.instance() or QApplication([])
@pytest.fixture
def cowork_tab(qt_app, tmp_path: Path, monkeypatch):
"""A real CoworkTab on a throwaway config, with ambient inputs neutralised."""
monkeypatch.setattr(chat_agent, "active_skills_text", lambda: "")
monkeypatch.setattr(chat_agent, "load_rules", lambda: "")
from cowork_local.core import audit_log
monkeypatch.setattr(audit_log, "AUDIT_DIR", tmp_path / "audit")
from cowork_local.ui.cowork_tab import CoworkTab
ctx = AppContext(AppConfig.load(tmp_path / "config.json"))
# Agent Security's prompt validation is ON by default and spends an EXTRA
# provider call reviewing the request before the agent loop starts (see
# core/agent_security.py::enforce_prompt). That is real behaviour - pinned
# by its own test below - but it would make every other test here script a
# turn that has nothing to do with what it is checking.
ctx.config.agent_security["enabled"] = False
return CoworkTab(ctx)
class _StubWorker:
"""The slice of ``core.worker.AgentWorker`` a job actually touches."""
def __init__(self) -> None:
self.events: List[Dict[str, Any]] = []
self.gates_requested = 0
self._cancelled = False
def emit_event(self, payload: Dict[str, Any]) -> None:
self.events.append(payload)
def is_cancelled(self) -> bool:
return self._cancelled
def new_gate(self, _mode: str, **_kwargs) -> Any:
self.gates_requested += 1
return None
def cancel(self) -> None:
self._cancelled = True
def _run_job(tab, worker, provider, text="hello", messages=None, out_dir=None):
"""Build the tab's job with ``provider`` pinned, then run it like the worker
thread would."""
tab.build_provider = lambda: provider # what routing/agent selection resolves to
job = tab.build_job(text, messages if messages is not None
else [{"role": "user", "content": text}], out_dir)
return job(worker)
def test_a_turn_runs_through_the_service_and_returns_history(cowork_tab, tmp_path):
provider = FakeProvider([ScriptedTurn(text="Hello from the fake.")])
worker = _StubWorker()
result = _run_job(cowork_tab, worker, provider, out_dir=tmp_path / "turn")
assert provider.call_count == 1
# Same return contract as before the refactor - _cleanup_turn reads both keys.
assert set(result) == {"messages", "turn_dir"}
assert [m["role"] for m in result["messages"]] == ["system", "user", "assistant"]
assert result["messages"][-1]["content"] == "Hello from the fake."
def test_the_widget_still_receives_the_legacy_event_dicts(cowork_tab, tmp_path):
"""The chat widgets consume dicts and are not migrated until EPIC R08, so
the typed events must render back into exactly what they already handle -
plus the new end-of-turn signal, which the if/elif dispatch ignores."""
provider = FakeProvider([ScriptedTurn(text="Hi")])
worker = _StubWorker()
_run_job(cowork_tab, worker, provider, out_dir=tmp_path / "turn")
assert [e["type"] for e in worker.events] == ["text", "assistant_done", "turn_completed"]
assert worker.events[0] == {"type": "text", "delta": "Hi"}
def test_the_turn_ignores_messages_added_after_it_was_submitted(cowork_tab, tmp_path):
"""The bug ConversationExecutionRequest exists to prevent: the panel keeps
appending to its own list while a turn is in flight."""
provider = FakeProvider([ScriptedTurn(text="ok")])
worker = _StubWorker()
live_messages = [{"role": "user", "content": "first question"}]
job_result = _run_job(cowork_tab, worker, provider,
messages=live_messages, out_dir=tmp_path / "turn")
# Simulate the user typing a second message DURING the turn by mutating the
# list the panel handed over. The already-sent conversation must not include it.
live_messages.append({"role": "user", "content": "typed while running"})
sent = provider.calls[0].messages
assert [m["content"] for m in sent if m["role"] == "user"] == ["first question"]
assert "typed while running" not in str(job_result["messages"])
def test_a_failing_turn_still_raises_so_the_worker_reports_it(cowork_tab, tmp_path):
"""core/worker.py turns an exception into the `failed` signal the chat panel
already handles; swallowing it here would show a successful turn with no
answer instead of an error."""
provider = FakeProvider([ScriptedTurn(error="gateway down"),
ScriptedTurn(error="gateway down")])
worker = _StubWorker()
with pytest.raises(Exception) as excinfo:
_run_job(cowork_tab, worker, provider, out_dir=tmp_path / "turn")
assert "gateway down" in str(excinfo.value)
# The error was still reported as an event before being re-raised.
assert any(e["type"] == "error" for e in worker.events)
def test_a_permission_gate_is_only_requested_when_the_workspace_asks_for_it(
cowork_tab, tmp_path, monkeypatch):
provider = FakeProvider([ScriptedTurn(text="ok"), ScriptedTurn(text="ok")])
worker = _StubWorker()
monkeypatch.setattr(cowork_tab.ctx, "project_confirm_commands", lambda: False)
_run_job(cowork_tab, worker, provider, out_dir=tmp_path / "a")
assert worker.gates_requested == 0
monkeypatch.setattr(cowork_tab.ctx, "project_confirm_commands", lambda: True)
_run_job(cowork_tab, worker, provider, out_dir=tmp_path / "b")
assert worker.gates_requested == 1
def test_a_tool_turn_writes_into_this_turns_own_output_folder(cowork_tab, tmp_path):
"""Turn isolation: each turn writes into its own directory so parallel turns
cannot clobber each other's files."""
provider = FakeProvider([
ScriptedTurn(tool_calls=[("save_file", {"filename": "n.md", "content": "x"})]),
ScriptedTurn(text="Saved."),
])
worker = _StubWorker()
turn_dir = tmp_path / "turn-1"
result = _run_job(cowork_tab, worker, provider, out_dir=turn_dir)
assert result["turn_dir"] == str(turn_dir)
assert [p.name for p in turn_dir.iterdir()] and turn_dir.exists()
assert any(e["type"] == "tool_result" and e["ok"] for e in worker.events)
def test_agent_security_still_reviews_the_request_before_the_turn_runs(
cowork_tab, tmp_path):
"""Characterisation, not a new behaviour: with Agent Security enabled (the
shipped default) a turn costs an EXTRA provider call, because the request is
reviewed against the rulebase before the agent loop starts.
Pinned here because it is invisible from the call site and easy to break -
routing a turn through the application layer must not skip the review.
"""
cowork_tab.ctx.config.agent_security["enabled"] = True
provider = FakeProvider([
ScriptedTurn(text="ALLOW"), # the security pre-flight review
ScriptedTurn(text="the answer"), # the turn itself
])
worker = _StubWorker()
result = _run_job(cowork_tab, worker, provider, out_dir=tmp_path / "turn")
assert provider.call_count == 2
assert result["messages"][-1]["content"] == "the answer"
+393
View File
@@ -0,0 +1,393 @@
"""Unit tests for EPIC R04: the turn snapshot, the typed events and the service.
The service tests run against the REAL engine (``core.chat_agent.run_cowork``)
driven by :class:`FakeProvider`, not against a stubbed runner. That is
deliberate: the whole point of R04 is that the service produces the same turn
the widget used to produce, and only an end-to-end path through the real engine
can show that. It still costs milliseconds - no Qt, no network, no disk beyond
a tmp folder.
"""
from __future__ import annotations
from pathlib import Path
from typing import Any, Dict, List
import pytest
from cowork_local.application.conversations import ConversationApplicationService
from cowork_local.core import chat_agent
from cowork_local.domain.agents import (
AssistantDoneEvent,
ConversationExecutionRequest,
ErrorEvent,
ReasoningChunkEvent,
TextChunkEvent,
ToolCallFinishedEvent,
ToolCallStartedEvent,
TurnCompletedEvent,
collect_text,
event_from_dict,
)
from tests.fakes import FakeProvider, FakeToolExecutor, ScriptedTurn
# --------------------------------------------------------------------------- #
# R04-T01 - the immutable request snapshot
# --------------------------------------------------------------------------- #
def test_the_snapshot_cannot_be_changed_by_the_caller_afterwards():
"""The motivating bug: the chat panel keeps appending to its own message
list while a turn runs, and the turn must not see those later messages."""
live_messages = [{"role": "user", "content": "first"}]
request = ConversationExecutionRequest.create("first", live_messages)
live_messages.append({"role": "user", "content": "typed while running"})
live_messages[0]["content"] = "edited"
assert len(request.messages) == 1
assert request.messages[0]["content"] == "first"
def test_message_list_hands_out_a_fresh_mutable_copy():
"""The engine appends assistant/tool messages to the list it is given, so a
copy is what keeps the snapshot immutable in practice, not just by
declaration."""
request = ConversationExecutionRequest.create("hi", [{"role": "user", "content": "hi"}])
first = request.message_list()
first.append({"role": "assistant", "content": "reply"})
assert len(request.message_list()) == 1
assert first is not request.message_list()
def test_with_model_produces_a_new_pinned_snapshot():
"""A routing switch must not mutate a request a turn may already be running."""
original = ConversationExecutionRequest.create("hi", provider="openai_compat", model="a")
routed = original.with_model("anthropic", "claude")
assert (original.provider, original.model) == ("openai_compat", "a")
assert (routed.provider, routed.model) == ("anthropic", "claude")
assert routed.turn_id == original.turn_id # same turn, different target
def test_every_turn_gets_its_own_id():
a = ConversationExecutionRequest.create("x")
b = ConversationExecutionRequest.create("x")
assert a.turn_id and b.turn_id and a.turn_id != b.turn_id
def test_run_to_completion_raises_the_step_ceiling():
interactive = ConversationExecutionRequest.create("x")
flow_step = ConversationExecutionRequest.create("x", run_to_completion=True)
assert interactive.effective_max_steps == 30
assert flow_step.effective_max_steps == 200
def test_permission_scope_always_keeps_update_plan():
"""update_plan has no side effects and drives the Plan panel; scoping it out
would break the UI rather than restrict a capability."""
request = ConversationExecutionRequest.create("x", allowed_tools=["read_file"])
assert request.allows_tool("read_file") is True
assert request.allows_tool("update_plan") is True
assert request.allows_tool("save_file") is False
# No scope at all means every enabled tool is allowed.
assert ConversationExecutionRequest.create("x").allows_tool("save_file") is True
# --------------------------------------------------------------------------- #
# R04-T02 - typed events and the legacy bridge
# --------------------------------------------------------------------------- #
@pytest.mark.parametrize("payload,expected", [
({"type": "text", "delta": "hi"}, TextChunkEvent),
({"type": "reasoning", "delta": "hmm"}, ReasoningChunkEvent),
({"type": "assistant_done", "content": "done"}, AssistantDoneEvent),
({"type": "tool_proposed", "id": "1", "name": "save_file"}, ToolCallStartedEvent),
({"type": "tool_result", "id": "1", "name": "save_file", "ok": True}, ToolCallFinishedEvent),
])
def test_legacy_emit_dicts_map_onto_typed_events(payload, expected):
assert isinstance(event_from_dict(payload), expected)
def test_an_unknown_event_tag_is_dropped_rather_than_raising():
"""The engine is still being refactored and may grow an event first. Losing
one bubble is survivable; aborting a turn that had succeeded is not."""
assert event_from_dict({"type": "something_new_in_r08"}) is None
@pytest.mark.parametrize("payload", [
{"type": "text", "delta": "hi"},
{"type": "tool_result", "id": "1", "name": "save_file", "ok": False, "output": "boom"},
{"type": "plan_set", "steps": [{"title": "a"}]},
{"type": "outputs_added", "paths": ["a.md"]},
])
def test_events_round_trip_back_into_the_legacy_shape(payload):
"""Existing widgets still consume dicts; an event must render back into
exactly what they already handle (EPIC R08 migrates them)."""
event = event_from_dict(payload)
rendered = event.to_dict()
assert rendered["type"] == payload["type"]
for key, value in payload.items():
assert rendered[key] == value
def test_events_are_immutable():
"""They cross a thread boundary; a consumer must not be able to edit one
out from under another consumer."""
event = TextChunkEvent("hi")
with pytest.raises(Exception):
event.delta = "changed" # type: ignore[misc]
def test_collect_text_returns_the_answer_without_the_reasoning():
events = [TextChunkEvent("Hel"), ReasoningChunkEvent("secret"), TextChunkEvent("lo")]
assert collect_text(events) == "Hello"
# --------------------------------------------------------------------------- #
# R04-T03 - the service, running the real engine
# --------------------------------------------------------------------------- #
@pytest.fixture
def isolated(monkeypatch, tmp_path: Path):
"""Same ambient isolation the characterization suite uses."""
monkeypatch.setattr(chat_agent, "active_skills_text", lambda: "")
monkeypatch.setattr(chat_agent, "load_rules", lambda: "")
from cowork_local.core import audit_log
monkeypatch.setattr(audit_log, "AUDIT_DIR", tmp_path / "audit")
return tmp_path
def _service(provider, **kwargs) -> ConversationApplicationService:
return ConversationApplicationService(lambda _p, _m: provider, **kwargs)
def _request(tmp_path: Path, prompt: str = "hi", **kwargs) -> ConversationExecutionRequest:
return ConversationExecutionRequest.create(
prompt, [{"role": "user", "content": prompt}],
output_dir=str(tmp_path / "out"), **kwargs)
def test_a_plain_turn_reports_text_and_a_final_answer(isolated):
provider = FakeProvider([ScriptedTurn(text="Hello there.")])
seen: List[Any] = []
result = _service(provider).run_turn(_request(isolated), on_event=seen.append)
assert result.ok is True
assert result.final_text == "Hello there."
assert [e.type for e in seen] == ["text", "assistant_done", "turn_completed"]
# The conversation coming back is what the caller persists as new history.
assert [m["role"] for m in result.messages] == ["system", "user", "assistant"]
def test_a_turn_always_ends_with_exactly_one_completion_event(isolated):
"""The end-of-turn signal the legacy engine never had: without it a
cancelled turn and a failed turn look identical to a consumer."""
provider = FakeProvider([ScriptedTurn(text="ok")])
seen: List[Any] = []
_service(provider).run_turn(_request(isolated), on_event=seen.append)
completions = [e for e in seen if isinstance(e, TurnCompletedEvent)]
assert len(completions) == 1
assert seen[-1] is completions[0]
def test_a_provider_failure_becomes_an_error_event_not_an_exception(isolated):
"""Callers run this on a worker thread; an escaped exception kills the
worker and the UI simply stops updating with nothing shown.
Two turns are scripted because the engine makes ONE silent recovery attempt
before giving up (core/code_agent.py::_call_provider_with_recovery) - the
service must report the failure only after that retry is also exhausted.
"""
provider = FakeProvider([ScriptedTurn(error="gateway exploded"),
ScriptedTurn(error="gateway exploded")])
seen: List[Any] = []
result = _service(provider).run_turn(_request(isolated), on_event=seen.append)
assert provider.call_count == 2 # original + one silent retry
assert result.ok is False
assert "gateway exploded" in result.error
assert any(isinstance(e, ErrorEvent) for e in seen)
assert isinstance(seen[-1], TurnCompletedEvent) # still a clean end
def test_a_transient_provider_failure_is_recovered_without_surfacing(isolated):
"""The engine's single retry must stay invisible: a turn that succeeds on
the second attempt reports no error at all."""
provider = FakeProvider([ScriptedTurn(error="connection reset"),
ScriptedTurn(text="recovered answer")])
result = _service(provider).run_turn(_request(isolated))
assert result.ok is True
assert result.final_text == "recovered answer"
assert not [e for e in result.events if isinstance(e, ErrorEvent)]
def test_a_cancelled_turn_is_reported_as_cancelled_not_failed(isolated):
provider = FakeProvider([], strict=True)
result = _service(provider).run_turn(_request(isolated), cancel=lambda: True)
assert result.cancelled is True
assert result.error == ""
assert provider.call_count == 0
assert result.events[-1].cancelled is True
def test_a_tool_turn_reports_the_full_lifecycle_and_writes_the_file(isolated):
provider = FakeProvider([
ScriptedTurn(tool_calls=[("save_file", {"filename": "note.md", "content": "# hi"})]),
ScriptedTurn(text="Saved."),
])
result = _service(provider).run_turn(_request(isolated, "make a note"))
assert [e.type for e in result.events] == [
"assistant_done", "tool_proposed", "tool_result",
"text", "assistant_done", "turn_completed",
]
finished = [e for e in result.events if isinstance(e, ToolCallFinishedEvent)]
assert finished[0].ok is True and finished[0].name == "save_file"
written = list((isolated / "out").iterdir())
assert len(written) == 1 and written[0].read_text(encoding="utf-8") == "# hi"
def test_external_tools_are_supplied_through_the_injected_tool_source(isolated):
executor = FakeToolExecutor(results={"ms365_send_mail": {"output": "sent"}})
provider = FakeProvider([
ScriptedTurn(tool_calls=[("ms365_send_mail", {"to": "a@b.c"})]),
ScriptedTurn(text="Mail sent."),
])
service = _service(provider, tool_source=lambda: (executor.specs(), executor))
result = service.run_turn(_request(isolated, "mail them"))
assert executor.call_names == ["ms365_send_mail"]
assert result.ok is True
def test_a_broken_tool_source_degrades_to_no_external_tools(isolated):
"""An MCP server that will not start must not stop the user from chatting -
the behaviour the chat panel already relies on today."""
def exploding_tool_source():
raise RuntimeError("mcp server did not start")
provider = FakeProvider([ScriptedTurn(text="still works")])
service = _service(provider, tool_source=exploding_tool_source)
result = service.run_turn(_request(isolated))
assert result.ok is True
assert result.final_text == "still works"
def test_a_consumer_that_raises_does_not_abort_the_turn(isolated):
"""A widget being torn down mid-turn must not take the turn with it."""
provider = FakeProvider([ScriptedTurn(text="answer")])
def bad_consumer(_event):
raise RuntimeError("widget already deleted")
result = _service(provider).run_turn(_request(isolated), on_event=bad_consumer)
assert result.ok is True
assert result.final_text == "answer"
def test_events_are_recorded_even_without_a_callback(isolated):
"""Headless callers (the scheduler) read the event list afterwards instead
of supplying a callback purely to collect it."""
provider = FakeProvider([ScriptedTurn(text="ok")])
result = _service(provider).run_turn(_request(isolated))
assert [e.type for e in result.events] == ["text", "assistant_done", "turn_completed"]
def test_the_request_permission_scope_reaches_the_engine(isolated):
"""A read-only step must literally not be offered a writing tool - the scope
has to survive the trip through the service or the restriction is silently
dropped."""
provider = FakeProvider([ScriptedTurn(text="ok")])
_service(provider).run_turn(_request(isolated, allowed_tools=["read_file"]))
advertised = set(provider.calls[0].tool_names)
assert "save_file" not in advertised
assert "update_plan" in advertised
def test_the_permission_gate_is_only_built_when_the_request_asks_for_it(isolated):
built: List[Any] = []
provider = FakeProvider([ScriptedTurn(text="ok"), ScriptedTurn(text="ok")])
service = _service(provider, gate_factory=lambda req: built.append(req) or object())
service.run_turn(_request(isolated))
assert built == []
service.run_turn(_request(isolated, confirm_commands=True))
assert len(built) == 1
def test_a_non_streamed_answer_still_produces_a_final_text(isolated):
"""A turn whose answer arrived without text events must still report an
answer - the scheduler writes it into output.md, and an empty string there
reads to the user as "(no output)"."""
provider = FakeProvider([ScriptedTurn(text="")])
service = _service(provider)
request = _request(isolated)
result = service.run_turn(request)
# run_cowork substitutes a placeholder for a reasoning-only reply; the
# service must surface that rather than an empty answer.
assert result.final_text != ""
# --------------------------------------------------------------------------- #
# Bridge completeness - the failure mode that motivated this test
# --------------------------------------------------------------------------- #
def test_every_event_the_engine_emits_has_a_typed_counterpart():
"""Scan the engine sources for ``emit({"type": "..."})`` tags and assert the
bridge knows all of them.
Written after a real miss: the first version of the bridge had no
``notice`` event, so routing turns through the service would have silently
swallowed Agent Security warnings and auto-compaction notices - the user
would simply never see that a request had been blocked. An unknown tag is
dropped by design (see event_from_dict), which is safe for a NEW event but
hides a forgotten one; this test is what turns that silence into a failure.
"""
import re
from pathlib import Path
from cowork_local.domain.agents.agent_event import EVENT_TYPES
repo = Path(__file__).resolve().parents[2]
sources = ["core/chat_agent.py", "core/code_agent.py", "core/agent_security.py",
"core/context_budget.py", "core/task_executors.py"]
emitted = set()
for rel in sources:
text = (repo / rel).read_text(encoding="utf-8")
# Only tags inside an emit(...) call; a bare {"type": "object"} in a
# JSON-Schema tool definition is not an event.
for match in re.finditer(r'emit(?:_and_autosave)?\(\s*\{\s*"type":\s*"([a-z_]+)"', text):
emitted.add(match.group(1))
missing = sorted(emitted - set(EVENT_TYPES))
assert not missing, (
f"engine emits {missing} but domain/agents/agent_event.py has no typed "
"counterpart - those events would be silently dropped by event_from_dict"
)