diff --git a/src/fi/alk/harness/contract.py b/src/fi/alk/harness/contract.py index b806786e3..a33ac2bc7 100644 --- a/src/fi/alk/harness/contract.py +++ b/src/fi/alk/harness/contract.py @@ -13,6 +13,7 @@ import collections.abc import json +import logging import os import re import shlex @@ -1483,6 +1484,10 @@ def adoptable(self, tool: str) -> bool: "RuntimeHttpSetupRequest", ) CONTRACT_FILE = "contract.json" +# What a reading was made from, saved beside its contract and outside the model: a build handed +# that reading is told apart from one that read the agent itself by this key alone, so every +# later save of the contract keeps it. +READING_RECORD = "reading_inputs" _SEQUENCES = ( list, tuple, @@ -1562,6 +1567,32 @@ def walk(annotation: Any, saved: Any, held: Any) -> None: return pairs +def _saved_record(path: Path) -> dict[str, Any]: + """The reading's record in the contract saved at ``path``, as ``{key: value}``; empty when + there is no file, no record in it, or it can't be read as a JSON object.""" + try: + raw = path.read_bytes() + except FileNotFoundError: + return {} + except OSError as exc: + # The save goes on without it, so a seeded build saves as an unseeded one: log the error + # in full. The file is the kit's own contract, not the customer's source. + logging.getLogger(__name__).warning( + "reading record not carried; class: %s", type(exc).__name__, exc_info=True + ) + return {} + # A contract that never held a record is not parsed again. + if READING_RECORD.encode() not in raw: + return {} + try: + saved = json.loads(raw.decode("utf-8")) + except (UnicodeDecodeError, ValueError, RecursionError): + return {} + if not isinstance(saved, dict) or READING_RECORD not in saved: + return {} + return {READING_RECORD: saved[READING_RECORD]} + + def write_contract(contract: AgentContract, destination: Path) -> AgentContract: """Save the contract to ``destination/contract.json``, its free text cleaned of URL credentials. @@ -1570,6 +1601,10 @@ def write_contract(contract: AgentContract, destination: Path) -> AgentContract: so a write that fails partway leaves the earlier contract as it was. Only then are the cleaned values copied back onto ``contract`` itself, in place: the stage and its tools hold that object, and a fresh one would leave them working from the raw text. It is returned too. + + The record of what a reading was made from is not part of the model. When the contract + already saved at ``destination`` holds one, the new save keeps its value, first, as the + reading put it; a save where none is held writes none. """ body = contract.model_dump() places: list[tuple[Any, Any, Any, str]] = [] @@ -1589,8 +1624,11 @@ def write_contract(contract: AgentContract, destination: Path) -> AgentContract: destination.mkdir(parents=True, exist_ok=True) path = destination / CONTRACT_FILE partial = destination / f".{CONTRACT_FILE}.{os.getpid()}.{threading.get_ident()}" + written = {**_saved_record(path), **body} try: - partial.write_text(json.dumps(body, indent=2, ensure_ascii=False), encoding="utf-8") + partial.write_text( + json.dumps(written, indent=2, ensure_ascii=False), encoding="utf-8" + ) # Synced before the move, so the move never publishes a file the disk does not hold. descriptor = os.open(partial, os.O_RDONLY) try: diff --git a/src/fi/alk/harness/seed_contract.py b/src/fi/alk/harness/seed_contract.py index cc930a721..afc7182ef 100644 --- a/src/fi/alk/harness/seed_contract.py +++ b/src/fi/alk/harness/seed_contract.py @@ -20,6 +20,7 @@ from typing import Any, NamedTuple from .agent_type_check import EXIT_REFUSED +from .contract import READING_RECORD from .failure_file import write_failure_file from .job import ( FailureDomain, @@ -31,7 +32,7 @@ ) from .reading_result import _CONTRACT_LIMIT_BYTES -RECORD_KEY = "reading_inputs" +RECORD_KEY = READING_RECORD SEED_ACCEPTED = ".futureagi-seed-accepted" SEED_CONTRACT_INVALID = "seed_contract_invalid" SEED_CONTRACT_TOO_LARGE = "seed_contract_too_large" diff --git a/tests/harness/test_contract_writer.py b/tests/harness/test_contract_writer.py index 2bcfa3e9a..5ff2404f3 100644 --- a/tests/harness/test_contract_writer.py +++ b/tests/harness/test_contract_writer.py @@ -305,6 +305,82 @@ def test_writing_a_written_contract_again_changes_nothing(tmp_path: Path) -> Non assert (tmp_path / "second" / "contract.json").read_bytes() == first +# What a reading saves beside its contract; any JSON value is kept as it was. +RECORD = { + "version": 1, + "source": {"kind": "archive", "archive_artifact_id": "upload-1"}, + "subfolder": "agents/a", + "evals_sha256": None, + "generic_harness_v1": True, + "odd": [0.1, "日本", {"nested": [None, False]}], +} + + +def _canon(value: Any) -> str: + return json.dumps(value, sort_keys=True, separators=(",", ":"), ensure_ascii=False) + + +def test_write_contract_keeps_the_record_saved_beside_the_contract( + tmp_path: Path, +) -> None: + held = _full_contract() + body = {**held.model_dump(), contract_module.READING_RECORD: RECORD} + (tmp_path / "contract.json").write_text(json.dumps(body), encoding="utf-8") + + write_contract(held, tmp_path) + saved = json.loads((tmp_path / "contract.json").read_text(encoding="utf-8")) + write_contract(AgentContract.model_validate(saved), tmp_path) + again = json.loads((tmp_path / "contract.json").read_text(encoding="utf-8")) + + for one in (saved, again): + assert next(iter(one)) == contract_module.READING_RECORD + assert _canon(one[contract_module.READING_RECORD]) == _canon(RECORD) + # Kept outside the model: neither the schema a model writes against nor the repair's hash + # of the contract sees it. + assert ( + contract_module.READING_RECORD + not in AgentContract.model_json_schema()["properties"] + ) + assert runtime_repair._contract_hash( + AgentContract.model_validate(again) + ) == runtime_repair._contract_hash(held) + + +@pytest.mark.parametrize( + "saved", + [ + None, + b"{}", + json.dumps({"agent": "a", "notes": "no reading_inputs here"}).encode(), + json.dumps({"runtime": {"reading_inputs": RECORD}}).encode(), + json.dumps(["reading_inputs", RECORD]).encode(), + b'{"reading_inputs": ', + b'\xff{"reading_inputs": {}}', + ], + ids=[ + "no_file", + "empty", + "named_in_prose", + "nested", + "not_an_object", + "not_json", + "not_utf8", + ], +) +def test_write_contract_writes_no_record_where_none_is_saved( + tmp_path: Path, saved: bytes | None +) -> None: + if saved is not None: + (tmp_path / "contract.json").write_bytes(saved) + write_contract(_full_contract(), tmp_path / "fresh") + + write_contract(_full_contract(), tmp_path) + + written = (tmp_path / "contract.json").read_bytes() + assert contract_module.READING_RECORD not in json.loads(written) + assert written == (tmp_path / "fresh" / "contract.json").read_bytes() + + def test_write_contract_cleans_the_callers_object_in_place_and_returns_it(tmp_path: Path) -> None: held = _full_contract() rules, tool = held.hard_constraints, held.tools[0] diff --git a/tests/harness/test_seed_contract.py b/tests/harness/test_seed_contract.py index 76c1bddf0..7bd393b88 100644 --- a/tests/harness/test_seed_contract.py +++ b/tests/harness/test_seed_contract.py @@ -18,13 +18,24 @@ from pydantic import BaseModel, ValidationError from fi.alk.harness import ( + amend, authoring_entrypoint, + authoring_runtime_validation, + builder, cli, + nudge, reading_entry, reading_result, + runtime_repair, seed_contract, + tools, +) +from fi.alk.harness.contract import ( + AgentContract, + Runtime, + RuntimeInterface, + write_contract, ) -from fi.alk.harness.contract import AgentContract from fi.alk.harness.framework_detection import detect_framework from fi.alk.harness.hosted_chat_entrypoint import HostedChatRuntime from fi.alk.harness.job import HarnessJob @@ -1423,6 +1434,53 @@ def test_a_source_model_already_there_survives_a_take_back(tmp_path, monkeypatch assert not (output / "contract.json").exists() +def test_a_build_after_a_take_back_saves_no_record(tmp_path, monkeypatch): + job = _job(source=UPLOAD) + seed = _place(tmp_path, _seed_bytes(job, _body(""))) + output = tmp_path / "authoring" + _setup_fails(tmp_path, monkeypatch, None) + + assert use_seed(seed, job, _source(tmp_path), output) == ("setup_failed", []) + + # A seeded contract saved elsewhere first leaves nothing behind: the build reads the agent + # itself, and its own save holds no record. + other = tmp_path / "other" + other.mkdir() + (other / "contract.json").write_bytes(_seed_bytes(job, _body(""))) + write_contract(AgentContract.model_validate(_body("")), other) + assert RECORD_KEY in json.loads((other / "contract.json").read_bytes()) + write_contract(AgentContract.model_validate(_body("")), output) + assert RECORD_KEY not in json.loads((output / "contract.json").read_bytes()) + + +def test_a_record_that_cant_be_read_back_is_logged(tmp_path, monkeypatch, caplog): + job = _job(source=UPLOAD) + output = tmp_path / "authoring" + output.mkdir() + saved = output / "contract.json" + saved.write_bytes(_seed_bytes(job)) + real = Path.read_bytes + monkeypatch.setattr( + Path, + "read_bytes", + lambda path: _raise(PermissionError()) if path == saved else real(path), + ) + + with caplog.at_level(logging.WARNING, logger="fi.alk.harness.contract"): + write_contract(AgentContract.model_validate(_body("")), output) + + assert [record.getMessage() for record in caplog.records] == [ + "reading record not carried; class: PermissionError" + ] + # With the trace: a seeded build saved as an unseeded one must show why. + [record] = caplog.records + assert record.exc_info is not None and record.exc_info[0] is PermissionError + + +def _raise(exc: BaseException): + raise exc + + def test_a_group_holding_a_validation_error_prints_nothing(capsys): import builtins @@ -1745,3 +1803,176 @@ def fails(destination, source, job): again = tmp_path / "again" assert use_seed(seed, job, _source(tmp_path), again) == ("setup_failed", []) assert not (again / "contract.json").exists() + + +# -- every save after the seed keeps the record ------------------------------------------------ + +# An uploaded folder is the agent itself: an archive job names no subfolder. +UPLOAD = {"kind": "archive", "archive_artifact_id": "upload-1"} +SOURCES = {"upload": (UPLOAD, ""), "github": (GITHUB, FOLDER)} +REPAIR = "generated_runtime_plan_invalid" + + +def _at(folder: str, name: str) -> str: + return f"{folder}/{name}" if folder else name + + +def _body(folder: str) -> dict: + runtime = {"language": "python", "workdir": folder or "."} + if folder: + runtime["source_subfolder"] = folder + # A tool entry for a tool the contract doesn't have, so the saves that drop one save. + stale = [{"tool": "gone", "mode": "unreachable"}] + return {**CONTRACT, "runtime": runtime, "tool_entrypoints": stale} + + +def _agent_source(tmp_path: Path, folder: str) -> Path: + source = tmp_path / "source" + agent = source / folder if folder else source + agent.mkdir(parents=True) + (agent / "agent.py").write_text("x = 1\n", encoding="utf-8") + (agent / "pyproject.toml").write_text( + "[project]\nname='agent'\nversion='1'\n", encoding="utf-8" + ) + (agent / "tools.py").write_text( + "def cancel():\n return 'ok'\n", encoding="utf-8" + ) + return source + + +def _repair_runtime(output: Path, source: Path, job: HarnessJob, folder: str) -> None: + runtime_repair.apply_runtime_plan_patch( + source, + output / "contract.json", + runtime_repair.RuntimePlanPatch( + base_contract_hash=runtime_repair._contract_hash(load(output)), + runtime=Runtime( + language="python", + workdir=folder, + command=["python", "agent.py"], + interface=RuntimeInterface(kind="http", port=8080, path="/chat"), + ), + evidence_paths=(_at(folder, "pyproject.toml"), _at(folder, "agent.py")), + diagnostic_codes=(REPAIR,), + summary="Run the HTTP service the folder declares.", + ), + allowed_diagnostic_codes={REPAIR}, + ) + + +def _promote_data_store( + output: Path, source: Path, job: HarnessJob, folder: str +) -> None: + candidate = output.parent / "candidate" + candidate.mkdir() + (candidate / "contract.json").write_text( + AgentContract( + agent="support", data_store={"kind": "in_memory"} + ).model_dump_json(), + encoding="utf-8", + ) + promoted = authoring_runtime_validation._promote_data_store( + load(output), output, candidate + ) + assert promoted == 0 + + +def _declare_tool(output: Path, source: Path, job: HarnessJob, folder: str) -> None: + server = builder.suite_server(output, [], load(output), source) + declare = next(one for one in server.tools if one.name == "declare_tool") + said = asyncio.run( + declare.handler( + { + "name": "cancel", + "description": "cancels", + "evidence_file": _at(folder, "tools.py"), + "evidence_quote": "def cancel():", + "why": "the agent has it", + } + ) + ) + assert not said.get("is_error") + + +def _add_rule(output: Path, source: Path, job: HarnessJob, folder: str) -> None: + added, _ = amend.add_rule(load(output), output, rule="Confirm first", why="asked") + assert added + + +RESAVES = { + "runtime_plan_repair": _repair_runtime, + "data_store_repair": _promote_data_store, + "submit_contract": lambda output, source, job, folder: tools._save_contract( + load(output), output + ), + "amend": _add_rule, + "declare_tool": _declare_tool, + "nudge": lambda output, source, job, folder: nudge.drop_stale_tool_references( + output + ), + "source_facts": lambda output, source, job, folder: ( + authoring_entrypoint._refresh_source_facts(job, source, output) + ), +} + + +@pytest.mark.parametrize( + ("made_from", "resave"), + [*(("upload", name) for name in RESAVES), ("github", "runtime_plan_repair")], + ids=[*(f"upload-{name}" for name in RESAVES), "github-runtime_plan_repair"], +) +def test_a_save_after_the_seed_keeps_the_record(tmp_path, made_from, resave): + source_entry, folder = SOURCES[made_from] + job = _job(source=source_entry) + seed = _place(tmp_path, _seed_bytes(job, _body(folder))) + source = _agent_source(tmp_path, folder) + output = tmp_path / "authoring" + assert use_seed(seed, job, source, output) == ("used", []) + installed = (output / "contract.json").read_bytes() + + RESAVES[resave](output, source, job, folder) + + raw = (output / "contract.json").read_bytes() + assert raw != installed + saved = json.loads(raw) + assert next(iter(saved)) == RECORD_KEY + assert seed_contract._canon(saved[RECORD_KEY]) == seed_contract._canon( + json.loads(installed)[RECORD_KEY] + ) + # What the platform checks: the saved contract is still the reading made for this job. + assert differs(saved, job) == [] + + +@pytest.mark.parametrize("resave", list(RESAVES)) +def test_a_save_of_a_contract_without_a_record_writes_none(tmp_path, resave): + job = _job(source=UPLOAD) + source = _agent_source(tmp_path, "") + output = tmp_path / "authoring" + output.mkdir() + unrecorded = _seed_bytes(job, _body(""), record=False) + (output / "contract.json").write_bytes(unrecorded) + + RESAVES[resave](output, source, job, "") + + raw = (output / "contract.json").read_bytes() + assert raw != unrecorded + assert RECORD_KEY not in json.loads(raw) + + +def test_a_readings_saves_hold_only_the_record_the_reading_put_in(tmp_path): + job = _job(source=UPLOAD) + source = _agent_source(tmp_path, "") + reading = tmp_path / "reading" + contract = AgentContract.model_validate(_body("")) + saved = reading / "contract.json" + + # The first save finds no file, and a save in the session after it finds no record. + for _ in range(2): + write_contract(contract, reading) + assert RECORD_KEY not in json.loads(saved.read_bytes()) + + assert document(job, source, reading) == 0 + assert json.loads(saved.read_bytes())[RECORD_KEY] == record_for(job) + + write_contract(contract, reading) + assert json.loads(saved.read_bytes())[RECORD_KEY] == record_for(job)