From 2aa357578992404240d27f9af7995ea2aa1ea3b6 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 18:21:18 +0530 Subject: [PATCH 1/2] fix(harness): keep reading_inputs through every re-save of a seeded contract (TH-8952) Every save of contract.json goes through write_contract, which wrote only the model's fields. reading_inputs isn't one, so a re-save after the seed (the runtime-plan repair, the data-store repair, submit_contract, the amend tools) dropped it, and the platform, which reads the key as "seeded", kept the reading's contract instead. write_contract now keeps the record held by the contract already saved at the destination, first and with the same value; a save where none is held writes none. No model field is added, so the schema and the repair's contract hash are unchanged. An unexpected error reading the old file is logged by class name and the save goes on. Tests run the seed on the upload and GitHub paths, then each re-save, and check the key survives; a contract without a record, a reading's own saves, and a build after a take-back gain none. TH-8952 Signed-off-by: khushalsonawat --- src/fi/alk/harness/contract.py | 39 ++++- src/fi/alk/harness/seed_contract.py | 3 +- tests/harness/test_contract_writer.py | 76 +++++++++ tests/harness/test_seed_contract.py | 230 +++++++++++++++++++++++++- 4 files changed, 345 insertions(+), 3 deletions(-) diff --git a/src/fi/alk/harness/contract.py b/src/fi/alk/harness/contract.py index b806786e3..f0df631b6 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,31 @@ 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; the class alone, as the seed's own skips are logged. + logging.getLogger(__name__).warning( + "reading record not carried; class: %s", type(exc).__name__ + ) + 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 +1600,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 +1623,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..c3fb047ae 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,50 @@ 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" + ] + + +def _raise(exc: BaseException): + raise exc + + def test_a_group_holding_a_validation_error_prints_nothing(capsys): import builtins @@ -1745,3 +1800,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) From 67a99e529a6035b4a26b7d94b0315a67e72a183e Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 12:45:28 +0530 Subject: [PATCH 2/2] fix(harness): log why a contract's record couldn't be read back, with its trace (TH-8952) A contract.json that can't be read back drops the reading record, so the save turns a seeded build into an unseeded one. Log the error with its traceback, not the class alone, and say why in the comment. Signed-off-by: khushalsonawat --- src/fi/alk/harness/contract.py | 5 +++-- tests/harness/test_seed_contract.py | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/fi/alk/harness/contract.py b/src/fi/alk/harness/contract.py index f0df631b6..a33ac2bc7 100644 --- a/src/fi/alk/harness/contract.py +++ b/src/fi/alk/harness/contract.py @@ -1575,9 +1575,10 @@ def _saved_record(path: Path) -> dict[str, Any]: except FileNotFoundError: return {} except OSError as exc: - # The save goes on without it; the class alone, as the seed's own skips are logged. + # 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__ + "reading record not carried; class: %s", type(exc).__name__, exc_info=True ) return {} # A contract that never held a record is not parsed again. diff --git a/tests/harness/test_seed_contract.py b/tests/harness/test_seed_contract.py index c3fb047ae..7bd393b88 100644 --- a/tests/harness/test_seed_contract.py +++ b/tests/harness/test_seed_contract.py @@ -1472,6 +1472,9 @@ def test_a_record_that_cant_be_read_back_is_logged(tmp_path, monkeypatch, caplog 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):