feat(R04): run every Cowork turn through ConversationApplicationService
R04-T03 — the turn lifecycle, extracted from `core/chat_agent.py::run_cowork` into `application/conversations/`. The 260-line body mixed the lifecycle (step budget, cancel checks, guard -> preview -> gate -> execute ordering, sandbox tidy-up) with the machinery doing each step, and reaching any of it meant standing up a Qt widget and a worker thread. It is now a plain object driven through two Protocols and six callables (`turn_runtime.py`), with the concrete `core/*` wiring confined to `core_runtime_adapter.py` — the same shape R03 used for routing. Faithful port, not an improvement pass: where the original had a quirk (the step-ceiling note only merges into the answer when the last message is the assistant's) the quirk is preserved and commented. R04-T04 — `ui/cowork_tab.py::build_job` no longer calls run_cowork. It captures the widget's state at submit time, builds the request via the new `cowork_turn_request.py` and executes it. `execute(..., messages=...)` hands the widget's own list over because `_reattach_running_turn` replays from it WHILE the worker appends and `_finalize_turn` slices it afterwards — a private list would break both silently. R04-T05 — `core/task_executors.py`'s cowork branch shares the same engine. All five unattended-run behaviours stay put (plan reminder, history_ready, History autosave per assistant message, timeout notice, plan_incomplete_reason), and `_unattended_prompt` now expresses the load-bearing prefix order in one readable call instead of three successive rebindings. Verification: 74 new tests (364 passed, 1 skipped overall; check_imports PASS). The two that matter most: - `test_conversation_service_parity.py` runs the same scripted turn through run_cowork AND the service and compares the event stream, the resulting conversation and the advertised tool list across 7 scenarios; - `test_task_executor_turn.py` was written BEFORE the migration and passed 8/8 against the old code, then unchanged against the new. Known: `ui/cowork_tab.py` (416 -> 455) and `core/task_executors.py` (476 -> 524) stay above the 400-LOC limit. Both were already over it before this change; bringing them under needs the R08 / R07 decompositions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,207 @@
|
||||
"""R04-T03 (c) — the service must behave exactly like ``run_cowork``.
|
||||
|
||||
The unit tests prove the loop follows the rules I wrote down. They cannot prove
|
||||
those rules are the ones the shipped runtime actually follows. This file does:
|
||||
each test scripts one provider, runs the SAME turn twice — once through
|
||||
``core/chat_agent.py::run_cowork``, once through
|
||||
``ConversationApplicationService`` wired by ``core_runtime_adapter`` — and
|
||||
compares the emitted event stream, the resulting conversation and the tool list
|
||||
the model was shown.
|
||||
|
||||
Anything the port got wrong (a missing event, a reordered guard, a different
|
||||
tool set, a changed message) fails here rather than in front of a user. The only
|
||||
allowed difference is the extra ``turn_completed`` event R04 introduces, which
|
||||
has no legacy consumer.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
|
||||
from cowork_local.application.conversations.core_runtime_adapter import (
|
||||
build_cowork_conversation_service,
|
||||
legacy_event_sink,
|
||||
)
|
||||
from cowork_local.core import chat_agent
|
||||
from cowork_local.domain.agents.conversation_execution_request import (
|
||||
ConversationExecutionRequest,
|
||||
)
|
||||
from cowork_local.tests.fakes.fake_provider import FakeProvider
|
||||
|
||||
_USER_TURN = [{"role": "user", "content": "make me a report"}]
|
||||
|
||||
|
||||
class _FakeGate:
|
||||
"""Stands in for ``core/permissions.py::PermissionGate``."""
|
||||
|
||||
def __init__(self, approve: bool) -> None:
|
||||
self.approve = approve
|
||||
self.requests: List[Dict[str, Any]] = []
|
||||
|
||||
def request(self, action: Dict[str, Any]) -> bool:
|
||||
self.requests.append(action)
|
||||
return self.approve
|
||||
|
||||
|
||||
def _normalise(events: List[Dict[str, Any]], out_dir: Path) -> List[Dict[str, Any]]:
|
||||
"""Replace the run's own output path with a placeholder.
|
||||
|
||||
The two runs write into different temp folders, so absolute paths in
|
||||
``tool_result``/``outputs_*`` events differ by construction. Everything else
|
||||
must match verbatim.
|
||||
"""
|
||||
marker, raw = "<OUT>", str(out_dir)
|
||||
|
||||
def scrub(value: Any) -> Any:
|
||||
if isinstance(value, str):
|
||||
return value.replace(raw, marker).replace(raw.replace("\\", "/"), marker)
|
||||
if isinstance(value, list):
|
||||
return [scrub(v) for v in value]
|
||||
if isinstance(value, dict):
|
||||
return {k: scrub(v) for k, v in value.items()}
|
||||
return value
|
||||
|
||||
return [scrub(e) for e in events]
|
||||
|
||||
|
||||
def _run_legacy(tmp_path: Path, provider: FakeProvider, *, allowed_tools=None,
|
||||
gate: Optional[_FakeGate] = None, max_steps: int = 30
|
||||
) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]], List[str]]:
|
||||
"""Run the turn through the existing ``run_cowork``."""
|
||||
out_dir = tmp_path / "legacy"
|
||||
out_dir.mkdir(parents=True, exist_ok=True)
|
||||
events: List[Dict[str, Any]] = []
|
||||
messages = [dict(m) for m in _USER_TURN]
|
||||
|
||||
chat_agent.run_cowork(
|
||||
provider, messages, out_dir, events.append, title="Report",
|
||||
security_config=None, allowed_tools=allowed_tools, gate=gate, max_steps=max_steps,
|
||||
)
|
||||
tool_names = [t.name for t in (provider.last_tools or [])]
|
||||
return _normalise(events, out_dir), messages, tool_names
|
||||
|
||||
|
||||
def _run_service(tmp_path: Path, provider: FakeProvider, *, allowed_tools=None,
|
||||
gate: Optional[_FakeGate] = None, max_steps: int = 30
|
||||
) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]], List[str]]:
|
||||
"""Run the same turn through the application service."""
|
||||
out_dir = tmp_path / "service"
|
||||
out_dir.mkdir(parents=True, exist_ok=True)
|
||||
events: List[Dict[str, Any]] = []
|
||||
|
||||
service = build_cowork_conversation_service(
|
||||
provider, out_dir, events.append, title="Report", security_config=None, gate=gate)
|
||||
request = ConversationExecutionRequest(
|
||||
turn_id="t1", session_id="s1",
|
||||
# run_cowork receives the user message already appended; the request
|
||||
# carries the history and this turn's prompt separately.
|
||||
messages=_USER_TURN[:-1], prompt=_USER_TURN[-1]["content"],
|
||||
output_dir=out_dir, allowed_tools=allowed_tools, max_steps=max_steps,
|
||||
gate_mode="confirm" if gate is not None else "auto",
|
||||
)
|
||||
result = service.execute(request, legacy_event_sink(events.append))
|
||||
|
||||
# The end-of-turn event is new in R04 and has no legacy counterpart.
|
||||
kept = [e for e in events if e.get("type") != "turn_completed"]
|
||||
tool_names = [t.name for t in (provider.last_tools or [])]
|
||||
return _normalise(kept, out_dir), list(result.messages), tool_names
|
||||
|
||||
|
||||
def _assert_parity(tmp_path: Path, script, *, approve: Optional[bool] = None, **kwargs) -> None:
|
||||
"""Script two identical providers, run both paths, compare everything."""
|
||||
legacy_provider, service_provider = FakeProvider(), FakeProvider()
|
||||
script(legacy_provider)
|
||||
script(service_provider)
|
||||
|
||||
legacy_gate = _FakeGate(approve) if approve is not None else None
|
||||
service_gate = _FakeGate(approve) if approve is not None else None
|
||||
|
||||
legacy_events, legacy_messages, legacy_tools = _run_legacy(
|
||||
tmp_path, legacy_provider, gate=legacy_gate, **kwargs)
|
||||
service_events, service_messages, service_tools = _run_service(
|
||||
tmp_path, service_provider, gate=service_gate, **kwargs)
|
||||
|
||||
assert service_events == legacy_events
|
||||
assert service_messages == legacy_messages
|
||||
assert service_tools == legacy_tools
|
||||
if legacy_gate is not None and service_gate is not None:
|
||||
assert [r["name"] for r in service_gate.requests] == \
|
||||
[r["name"] for r in legacy_gate.requests]
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Scenarios.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_a_plain_answer_turn_behaves_identically(tmp_path: Path) -> None:
|
||||
def script(provider: FakeProvider) -> None:
|
||||
provider.queue_response(content="Here you go.", chunks=["Here ", "you go."])
|
||||
|
||||
_assert_parity(tmp_path, script)
|
||||
|
||||
|
||||
def test_a_save_file_turn_behaves_identically(tmp_path: Path) -> None:
|
||||
def script(provider: FakeProvider) -> None:
|
||||
provider.queue_response(
|
||||
content="Writing it.",
|
||||
tool_calls=[{"id": "c1", "name": "save_file",
|
||||
"arguments": {"filename": "report.md", "content": "# Report\n"}}],
|
||||
)
|
||||
provider.queue_response(content="Saved.")
|
||||
|
||||
_assert_parity(tmp_path, script)
|
||||
|
||||
|
||||
def test_an_update_plan_turn_behaves_identically(tmp_path: Path) -> None:
|
||||
def script(provider: FakeProvider) -> None:
|
||||
provider.queue_response(
|
||||
content="Planning.",
|
||||
tool_calls=[{"id": "c1", "name": "update_plan",
|
||||
"arguments": {"steps": [{"title": "Draft", "status": "running"},
|
||||
{"title": "Ship", "status": "pending"}]}}],
|
||||
)
|
||||
provider.queue_response(content="Done.")
|
||||
|
||||
_assert_parity(tmp_path, script)
|
||||
|
||||
|
||||
def test_a_reasoning_only_reply_behaves_identically(tmp_path: Path) -> None:
|
||||
def script(provider: FakeProvider) -> None:
|
||||
provider.queue_response(content="", reasoning="thinking hard")
|
||||
|
||||
_assert_parity(tmp_path, script)
|
||||
|
||||
|
||||
def test_restricting_the_tool_scope_advertises_the_same_tools(tmp_path: Path) -> None:
|
||||
def script(provider: FakeProvider) -> None:
|
||||
provider.queue_response(content="ok")
|
||||
|
||||
_assert_parity(tmp_path, script, allowed_tools=["save_file"])
|
||||
|
||||
|
||||
def test_a_rejected_command_behaves_identically(tmp_path: Path) -> None:
|
||||
# The security-critical path: the gate says no, so the command must never
|
||||
# run and the model must read back the same refusal in both designs.
|
||||
def script(provider: FakeProvider) -> None:
|
||||
provider.queue_response(
|
||||
content="Running it.",
|
||||
tool_calls=[{"id": "c1", "name": "run_command",
|
||||
"arguments": {"command": "echo hi"}}],
|
||||
)
|
||||
provider.queue_response(content="Understood.")
|
||||
|
||||
_assert_parity(tmp_path, script, approve=False)
|
||||
|
||||
|
||||
def test_hitting_the_step_ceiling_behaves_identically(tmp_path: Path) -> None:
|
||||
# The model never stops calling tools, so both paths must stop at the same
|
||||
# place and say so the same way.
|
||||
def script(provider: FakeProvider) -> None:
|
||||
for i in range(4):
|
||||
provider.queue_response(
|
||||
content=f"step {i}",
|
||||
tool_calls=[{"id": f"c{i}", "name": "save_file",
|
||||
"arguments": {"filename": f"f{i}.md", "content": "x"}}],
|
||||
)
|
||||
|
||||
_assert_parity(tmp_path, script, max_steps=2)
|
||||
Reference in New Issue
Block a user