diff --git a/src/fi/alk/harness/call_runner.py b/src/fi/alk/harness/call_runner.py index 653772103..26f8fb250 100644 --- a/src/fi/alk/harness/call_runner.py +++ b/src/fi/alk/harness/call_runner.py @@ -447,7 +447,7 @@ def _read_scenario_document(bundle_dir: Path, scenario_key: str) -> dict[str, An # Edits a call actually plays; everything else is as sealed. -_EDITABLE_FIELDS = ("background_noise",) +_EDITABLE_FIELDS = ("background_noise", "max_turns") _EDITABLE_PERSONA_FIELDS = ( "personality", "communication_style", diff --git a/src/fi/alk/harness/callback_http_adapter.py b/src/fi/alk/harness/callback_http_adapter.py index 39c4fa820..e78fa969e 100644 --- a/src/fi/alk/harness/callback_http_adapter.py +++ b/src/fi/alk/harness/callback_http_adapter.py @@ -17,6 +17,9 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import Any +# Main thread only: with livekit installed this import registers plugins. +from fi.simulate import AgentInput + def _load_callback() -> Any: target = os.environ["ALK_CALLBACK_ENTRYPOINT"] @@ -46,8 +49,6 @@ def _json_body(value: Any) -> dict[str, Any]: def _invoke(payload: dict[str, Any]) -> dict[str, Any]: - from fi.simulate import AgentInput - result = CALLBACK(AgentInput.model_validate(payload)) if inspect.isawaitable(result): result = asyncio.run(result) diff --git a/src/fi/alk/harness/scenario_tools.py b/src/fi/alk/harness/scenario_tools.py index 488cbac3a..28ed1260d 100644 --- a/src/fi/alk/harness/scenario_tools.py +++ b/src/fi/alk/harness/scenario_tools.py @@ -250,6 +250,14 @@ def persona_vocabulary_note() -> str: ) +# A suite is never saved empty, so an emptied list changes nothing on disk until one is submitted. +_EMPTIED = ( + "Every scenario is dropped from this session's list, but nothing is saved yet: a suite is never " + "saved empty, so the saved scenarios stay until you submit at least one new scenario, which " + "then replaces them. Do not tell the person the suite is empty or saved." +) + + def write_scenarios( scenarios: list[Scenario], destination: Path, catalogue: Catalogue | None = None ) -> Path: @@ -1854,19 +1862,26 @@ async def suite_progress(args: dict[str, Any]) -> dict[str, Any]: @tool( "drop_scenario", - "Remove a scenario by name, or all of them with name '*'.", - schema({"name": str}, ["name"]), + "Remove a scenario by name. Removing every scenario takes name '*' with " + "everything=true, and only when the person asked to start the suite over.", + schema({"name": str, "everything": bool}, ["name"]), ) async def drop_scenario(args: dict[str, Any]) -> dict[str, Any]: name = str(args.get("name") or "") if name == "*": + if args.get("everything") is not True: + return _err( + "Dropping every scenario needs everything=true. Do that only when the " + "person asked to start the suite over; otherwise drop scenarios by name." + ) kept.clear() - write_scenarios(kept, destination, catalogue) - return _ok("all scenarios dropped") + return _ok(_EMPTIED) before = len(kept) kept[:] = [one for one in kept if one.name != name] if len(kept) == before: return _err(f"no scenario called {name!r}") + if not kept: + return _ok(_EMPTIED) write_scenarios(kept, destination, catalogue) return _ok(f"{name} dropped. {len(kept)} left") diff --git a/src/fi/alk/harness/skills/write-scenarios/SKILL.md b/src/fi/alk/harness/skills/write-scenarios/SKILL.md index c68cc9c58..9644219e4 100644 --- a/src/fi/alk/harness/skills/write-scenarios/SKILL.md +++ b/src/fi/alk/harness/skills/write-scenarios/SKILL.md @@ -493,7 +493,7 @@ called. Assert on what the world looks like afterwards and on what the caller wa | `try_calls` | Runs calls against a **throwaway copy** of the world and shows the state they leave. This is how you work out a solution and what its checks should assert. Nothing you do here is visible to anybody else. | | `add_sub_goal` | Adds a named thing this agent can be checked on, with its check in code. | | `submit_scenario` | Keeps one scenario, after validation and the three gates. | -| `drop_scenario` | Removes one by name, or all of them with `*`. | +| `drop_scenario` | Removes one by name; all of them only with `*` and `everything=true`. A suite is never saved empty: the saved scenarios stay until you submit a new one, which replaces them. | | `aim_for` | Sets how many scenarios are wanted. Needed when reopening an existing suite to add more, because the target starts at what is already there. Not for saving a suite nobody asked for. | | `save_scenarios` | Finishes the suite. Reports what is off about it as a whole. | | `amend_contract`, `add_rule`, `drop_rule`, `fix_tool` | Correct the contract when it is wrong. See the last section. | diff --git a/tests/harness/test_callback_http_adapter.py b/tests/harness/test_callback_http_adapter.py new file mode 100644 index 000000000..cb21eb3ae --- /dev/null +++ b/tests/harness/test_callback_http_adapter.py @@ -0,0 +1,73 @@ +import json +import os +import socket +import subprocess +import sys +import time +import urllib.request +from pathlib import Path + +import pytest + +ADAPTER = ( + Path(__file__).resolve().parents[2] / "src/fi/alk/harness/callback_http_adapter.py" +) + + +def _free_port() -> int: + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + return probe.getsockname()[1] + + +@pytest.fixture +def adapter(tmp_path): + (tmp_path / "chat_agent.py").write_text( + "def reply(agent_input):\n" + " return {'content': 'thread ' + agent_input.thread_id}\n", + encoding="utf-8", + ) + # Bundles embed the adapter's source and run it from the bundle, not from the package. + script = tmp_path / "adapter.py" + script.write_text(ADAPTER.read_text(encoding="utf-8"), encoding="utf-8") + port = _free_port() + process = subprocess.Popen( + [sys.executable, str(script)], + env={ + **os.environ, + "ALK_CALLBACK_ENTRYPOINT": "chat_agent:reply", + "PORT": str(port), + "PYTHONPATH": str(tmp_path), + }, + stderr=subprocess.PIPE, + ) + base = f"http://127.0.0.1:{port}" + deadline = time.monotonic() + 30 + while time.monotonic() < deadline: + try: + urllib.request.urlopen(f"{base}/health", timeout=1) + break + except OSError: + if process.poll() is not None: + pytest.fail(process.stderr.read().decode()) + time.sleep(0.2) + yield base + process.terminate() + process.wait(timeout=10) + + +def test_a_callback_answers_from_the_request_thread(adapter): + """The request thread is not the main thread; nothing imported there may require it.""" + body = json.dumps( + { + "thread_id": "t1", + "messages": [{"role": "user", "content": "hello"}], + "new_message": {"role": "user", "content": "hello"}, + } + ).encode() + request = urllib.request.Request( + f"{adapter}/invoke", data=body, headers={"Content-Type": "application/json"} + ) + with urllib.request.urlopen(request, timeout=30) as response: + assert response.status == 200 + assert json.loads(response.read())["content"] == "thread t1" diff --git a/tests/harness/test_scenario_edits.py b/tests/harness/test_scenario_edits.py index 21423d461..d1300b6c7 100644 --- a/tests/harness/test_scenario_edits.py +++ b/tests/harness/test_scenario_edits.py @@ -18,9 +18,14 @@ "scenario_edits": { "book-ride": { "background_noise": "street", + "max_turns": 4, "tests": "unused by a call", "instruction": "ignored", - "persona": {"personality": "impatient", "name": "ignored"}, + "persona": { + "personality": "impatient", + "languages": ["English", "Hindi"], + "name": "ignored", + }, } } } @@ -30,7 +35,13 @@ def test_edits_replace_only_what_a_call_plays(): merged = _with_scenario_edits(SEALED, EDITS, "book-ride") assert merged["background_noise"] == "street" - assert merged["persona"] == {"name": "Asha", "accent": "Indian", "personality": "impatient"} + assert merged["persona"] == { + "name": "Asha", + "accent": "Indian", + "personality": "impatient", + "languages": ["English", "Hindi"], + } + assert merged["max_turns"] == 4 assert merged["tests"] == "books the ride" assert merged["instruction"] == "book a ride" assert SEALED["background_noise"] == "office" @@ -47,7 +58,10 @@ def test_a_voice_call_plays_the_edited_place(): def test_a_chat_caller_is_the_edited_persona(): merged = _with_scenario_edits(SEALED, EDITS, "book-ride") - assert _conversation_scenario(merged).persona.personality == "impatient" + scenario = _conversation_scenario(merged) + + assert scenario.persona.personality == "impatient" + assert scenario.max_turns == 4 def test_no_edits_leave_the_sealed_document(): diff --git a/tests/test_harness.py b/tests/test_harness.py index 16baa5945..ec032e377 100644 --- a/tests/test_harness.py +++ b/tests/test_harness.py @@ -7978,3 +7978,34 @@ def test_sub_goals_no_scenario_names_are_the_ones_left_out(): assert unused_sub_goals(catalogue, kept) == ["test_probe_sub_goal_ast"] +def test_dropping_every_scenario_needs_an_explicit_everything(tmp_path): + import asyncio + + from mcp.types import CallToolRequestParams + + from fi.alk.harness import scenario_tools as module + + from fi.alk.harness.scenario_tools import accept_scenario + + root, contract, catalogue = _built_environment(tmp_path) + saved = [] + accept_scenario(_delta(), world_root=root, catalogue=catalogue, kept=saved) + module.write_scenarios(saved, root, catalogue) + server, _kept = module.scenario_tools(contract, root, root, wanted=1) + instance = _instance(server) + + async def drop(arguments): + handler = _request_handler(instance, "tools/call") + answer = await handler.handler( + None, CallToolRequestParams(name="drop_scenario", arguments=arguments) + ) + return answer.isError, answer.content[0].text + + refused = asyncio.run(drop({"name": "*"})) + by_name = asyncio.run(drop({"name": saved[0].name})) + everything = asyncio.run(drop({"name": "*", "everything": True})) + + assert refused[0] and "everything=true" in refused[1] + assert not by_name[0] and "nothing is saved yet" in by_name[1] + assert not everything[0] and "nothing is saved yet" in everything[1] + assert [one.name for one in module.load_scenarios(root)] == [saved[0].name]