From 65aece2b8925a6ea18d7178e476dbf5528652903 Mon Sep 17 00:00:00 2001 From: jitagent Dev-1 Date: Thu, 5 Nov 2026 01:30:52 +0000 Subject: [PATCH 1/2] Explain a broken case directory in one line instead of a traceback `jitagent verify` read case.json and trusted it: a missing file, invalid JSON, a missing key or a typo key each came out as the standard-library exception that happened to be raised inside Case.load (16-25 stderr lines, exit code 1 - the same code that means "the implementation failed verification"). Case.load now raises CaseError naming the file and the reason, and cmd_verify turns it into one line on stderr and exit code 2. Valid case directories render exactly as before. --- CHANGELOG.md | 12 ++ src/jitagent/cases.py | 106 ++++++++++++++++- src/jitagent/cli.py | 8 +- tests/test_verify_case_errors.py | 192 +++++++++++++++++++++++++++++++ 4 files changed, 310 insertions(+), 8 deletions(-) create mode 100644 tests/test_verify_case_errors.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b26d28b..42dffb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,18 @@ This project follows [Semantic Versioning](https://semver.org/). While the major version is 0, the API may change in a minor release. +## [Unreleased] + +### Fixed + +- `jitagent verify ` now answers a broken case directory in one sentence and + **exit code 2**, where it used to print a traceback: a missing or unreadable + `case.json`, invalid JSON, a top level that is not an object, a missing + `spec`/`examples`/`expect` (or `expect.level`), an unknown key in `spec` or an + example, an `examples` value that is not a list, an example that is not an object or + lacks `input`/`output`, a missing implementation file, and a file that is not UTF-8 + (the classic Windows save). Valid cases render exactly as before. + ## [0.0.1] — 2026-09-16 First release. The loop round-trips: **a sentence goes in, code comes out, and you fetch diff --git a/src/jitagent/cases.py b/src/jitagent/cases.py index fb7a1d1..bbc0287 100644 --- a/src/jitagent/cases.py +++ b/src/jitagent/cases.py @@ -5,9 +5,49 @@ import json from dataclasses import dataclass from pathlib import Path +from typing import Any from .types import Example, Spec +# The keys a case file may carry, taken from the dataclasses that consume them. A typo +# here used to surface as `TypeError: Spec.__init__() got an unexpected keyword argument` +# from inside `Case.load`, which is a traceback where a sentence belongs. +_SPEC_KEYS = ("intent", "param_schema", "return_schema", "entry", "timeout_ms", "mem_mb") +_SPEC_REQUIRED = ("intent", "param_schema", "return_schema") +_EXAMPLE_KEYS = ("input", "output", "note", "boundary", "origin", "assumes") + +_KINDS = {dict: "object", list: "array", str: "string", bool: "boolean", + type(None): "null", int: "number", float: "number"} + + +class CaseError(RuntimeError): + """A case directory that cannot be read as a case. + + `verify` is the first command anyone runs, against files they wrote by hand, so a + bad `case.json` has to come back as one sentence and exit 2 — not as a traceback + out of the JSON module, a dict lookup, or a dataclass constructor. + """ + + +def _kind(value: Any) -> str: + return _KINDS.get(type(value), type(value).__name__) + + +def _article(kind: str) -> str: + return "an" if kind[0] in "aeiou" else "a" + + +def _read(path: Path) -> str: + try: + # Explicit: the message below says "not UTF-8", so that has to be the rule even + # when the platform default is something else. + return path.read_text(encoding="utf-8") + except UnicodeDecodeError as e: + raise CaseError( + f"cannot read {str(path)!r}: not UTF-8 text ({e})") from None + except OSError as e: + raise CaseError(f"cannot read {str(path)!r}: {e.strerror or e}") from None + @dataclass class Case: @@ -22,15 +62,69 @@ class Case: @classmethod def load(cls, d: Path) -> "Case": - meta = json.loads((d / "case.json").read_text()) + meta_path = d / "case.json" + raw = _read(meta_path) + try: + meta = json.loads(raw) + except json.JSONDecodeError as e: + raise CaseError(f"{str(meta_path)!r} is not valid JSON: {e}") from None + if not isinstance(meta, dict): + kind = _kind(meta) + raise CaseError( + f'{str(meta_path)!r} must hold a JSON object with "spec", "examples" and ' + f'"expect", not {_article(kind)} {kind}') + for key in ("spec", "examples", "expect"): + if key not in meta: + raise CaseError(f"{str(meta_path)!r} is missing {json.dumps(key)}") + + spec_raw = meta["spec"] + if not isinstance(spec_raw, dict): + raise CaseError(f'{str(meta_path)!r}: "spec" must be an object') + for key in spec_raw: + if key not in _SPEC_KEYS: + raise CaseError( + f'{str(meta_path)!r}: "spec" has an unknown key {json.dumps(key)}') + for key in _SPEC_REQUIRED: + if key not in spec_raw: + raise CaseError( + f'{str(meta_path)!r}: "spec" is missing {json.dumps(key)}') + + examples_raw = meta["examples"] + if not isinstance(examples_raw, list): + raise CaseError(f'{str(meta_path)!r}: "examples" must be a list of examples') + examples = [] + for i, entry in enumerate(examples_raw): + if not isinstance(entry, dict): + raise CaseError(f"{str(meta_path)!r}: example {i} must be an object") + for key in entry: + if key not in _EXAMPLE_KEYS: + raise CaseError( + f"{str(meta_path)!r}: example {i} has an unknown key " + f"{json.dumps(key)}") + missing = [k for k in ("input", "output") if k not in entry] + if missing: + raise CaseError( + f'{str(meta_path)!r}: example {i} needs both "input" and "output" ' + f"(missing {', '.join(repr(k) for k in missing)})") + examples.append(Example.from_dict(entry)) + + expect = meta["expect"] + if not isinstance(expect, dict) or "level" not in expect: + raise CaseError( + f'{str(meta_path)!r} needs "expect" to be an object with a "level"') + + impl = meta.get("impl", "impl.py") + if not isinstance(impl, str): + raise CaseError(f'{str(meta_path)!r}: "impl" must be a string') + source = _read(d / impl) return cls( name=meta.get("name", d.name), why=meta.get("why", ""), - source=(d / meta.get("impl", "impl.py")).read_text(), - spec=Spec(**meta["spec"]), - examples=[Example(**e) for e in meta["examples"]], - expect_level=meta["expect"]["level"], - expect_failing=meta["expect"].get("failing_gates", []), + source=source, + spec=Spec(**spec_raw), + examples=examples, + expect_level=expect["level"], + expect_failing=expect.get("failing_gates", []), path=d, ) diff --git a/src/jitagent/cli.py b/src/jitagent/cli.py index 1cec36b..7929176 100644 --- a/src/jitagent/cli.py +++ b/src/jitagent/cli.py @@ -7,7 +7,7 @@ import json as _json -from .cases import Case, load_all +from .cases import Case, CaseError, load_all from .types import Example from .verify import Thresholds, verify @@ -29,7 +29,11 @@ def _print_details(report, verbose: bool) -> None: def cmd_verify(args) -> int: - case = Case.load(Path(args.case)) + try: + case = Case.load(Path(args.case)) + except CaseError as e: + print(e, file=sys.stderr) + return 2 report = verify(case.source, case.spec, case.examples) print(f"== {case.name} ==") print(report.render()) diff --git a/tests/test_verify_case_errors.py b/tests/test_verify_case_errors.py new file mode 100644 index 0000000..8fd9d59 --- /dev/null +++ b/tests/test_verify_case_errors.py @@ -0,0 +1,192 @@ +"""`jitagent verify` must refuse an unusable case directory, not dump a traceback. + +`Case.load` is a straight pipeline: read `case.json`, `Spec(**meta["spec"])`, +`Example(**e)`, `meta["expect"]["level"]`. Every ordinary authoring mistake — a missing +file, a trailing comma, a typo in a key — therefore surfaces as whatever exception the +standard library raises: a 16–25 line traceback, empty stdout, exit code 1. But 1 is also +what `verify` returns for "the implementation failed verification", so a crash and a +REJECTED verdict are indistinguishable to a script or a CI job. + +The contract pinned here: exit code **2**, exactly **one** line on stderr, nothing on +stdout, no traceback, and the line names the file it could not use with a repr-quoted +path plus the reason. The assertions stop at the agreed reason phrase rather than the +whole sentence, so punctuation beyond it stays Dev-1's to choose. + +Two tests here pass on the unfixed tree as well — they are pins, not coverage: +a valid case still verifies (rc 0) and a wrong implementation still reports REJECTED +(rc 1). They keep "rc 2" meaning "bad input" rather than swallowing the verdict. +""" +import json + +import pytest + +from jitagent.cli import main + +ADD = 'def solve(params, ctx):\n return params["a"] + params["b"]\n' +WRONG = 'def solve(params, ctx):\n return params["a"] - params["b"]\n' + + +def valid_meta(): + """A fresh case.json body that verifies: one example, integer in and out.""" + return { + "name": "add", + "why": "the happy path: two integers", + "impl": "impl.py", + "spec": { + "intent": "add two integers", + "param_schema": {"type": "object", + "properties": {"a": {"type": "integer"}, + "b": {"type": "integer"}}, + "required": ["a", "b"]}, + "return_schema": {"type": "integer"}, + "entry": "solve", + }, + "examples": [{"input": {"a": 1, "b": 2}, "output": 3}], + "expect": {"level": "VERIFIED"}, + } + + +def write_case(root, meta=None, text=None, impl=ADD, impl_name="impl.py"): + """Lay out a case directory. `meta=None, text=None` means: no case.json.""" + d = root / "case" + d.mkdir() + if impl is not None: + (d / impl_name).write_text(impl) + if text is not None: + (d / "case.json").write_text(text) + elif meta is not None: + (d / "case.json").write_text(json.dumps(meta, indent=2)) + return d + + +def run_verify(tmp_path, case_dir): + return main(["--home", str(tmp_path / "registry"), "verify", str(case_dir)]) + + +def assert_refused(capsys, rc, path, phrase): + """The shape every bad case file has to produce; returns the one line, for detail.""" + out, err = capsys.readouterr() + assert rc == 2, "a case file jitagent cannot use is not a verification result" + assert out == "", "nothing belongs on stdout when the case itself is broken" + assert "Traceback" not in err + lines = [ln for ln in err.splitlines() if ln.strip()] + assert len(lines) == 1, f"one diagnostic line expected, got {len(lines)}: {err!r}" + line = lines[0] + assert repr(str(path)) in line, "the message must name the file it could not use" + assert phrase in line, f"{phrase!r} not in {line!r}" + return line + + +# --- the file itself is unreadable --------------------------------------------------- +def test_a_missing_case_file_is_a_usage_error(tmp_path, capsys): + d = write_case(tmp_path) # impl.py only: no case.json + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", "cannot read") + assert "No such file or directory" in line + + +def test_a_case_file_that_is_not_utf8_is_a_usage_error(tmp_path, capsys): + """UTF-16 is what an editor on another platform hands you. `read_text` raises + `UnicodeDecodeError` — a `ValueError`, not the `OSError` the other file errors are — + so it needs its own branch, and the message has to say the encoding is the problem.""" + d = write_case(tmp_path) # impl.py only: case.json comes next + (d / "case.json").write_text(json.dumps(valid_meta()), encoding="utf-16") + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", "not UTF-8") + assert "cannot read" in line + + +def test_invalid_json_names_the_line_and_column(tmp_path, capsys): + d = write_case(tmp_path, text='{"name": "add",\n "spec": {},') + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + "is not valid JSON") + # The decoder's text is what tells an author where to look; keep it. + assert "line" in line.lower() and "column" in line.lower() + + +def test_a_json_array_is_not_a_case(tmp_path, capsys): + d = write_case(tmp_path, text="[1, 2, 3]") + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + "must hold a JSON object") + assert "array" in line + + +# --- required structure --------------------------------------------------------------- +@pytest.mark.parametrize("key", ["spec", "examples"]) +def test_a_missing_top_level_key_is_named(tmp_path, capsys, key): + meta = valid_meta() + del meta[key] + d = write_case(tmp_path, meta=meta) + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", "is missing") + assert f'"{key}"' in line + + +def test_an_unknown_key_in_the_spec_is_named(tmp_path, capsys): + meta = valid_meta() + meta["spec"]["timeout"] = 1000 # the real key is timeout_ms + d = write_case(tmp_path, meta=meta) + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + '"spec" has an unknown key "timeout"') + + +def test_an_unknown_key_in_an_example_is_named(tmp_path, capsys): + meta = valid_meta() + meta["examples"][0]["notes"] = "a typo for note" + d = write_case(tmp_path, meta=meta) + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + 'example 0 has an unknown key "notes"') + + +def test_examples_that_are_not_a_list_are_refused(tmp_path, capsys): + meta = valid_meta() + meta["examples"] = {"input": {"a": 1, "b": 2}, "output": 3} + d = write_case(tmp_path, meta=meta) + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + '"examples" must be a list of examples') + + +@pytest.mark.parametrize("key", ["input", "output"]) +def test_an_example_needs_both_sides(tmp_path, capsys, key): + meta = valid_meta() + del meta["examples"][0][key] + d = write_case(tmp_path, meta=meta) + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + "needs both") + assert repr(key) in line, "the message must say which side is missing" + + +def test_expect_must_be_an_object_with_a_level(tmp_path, capsys): + meta = valid_meta() + meta["expect"] = "VERIFIED" # a string is the natural typo + d = write_case(tmp_path, meta=meta) + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + '"expect" to be an object with a "level"') + + +def test_a_named_impl_file_that_is_missing_is_reported(tmp_path, capsys): + meta = valid_meta() + meta["impl"] = "missing.py" + d = write_case(tmp_path, meta=meta, impl=None) # only impl.py is absent... + line = assert_refused(capsys, run_verify(tmp_path, d), d / "missing.py", + "cannot read") + assert "No such file or directory" in line + + +# --- pins: the verdict path is unchanged --------------------------------------------- +def test_a_valid_case_still_verifies(tmp_path, capsys): + """**Passes on the unfixed tree too** — a pin. A loadable case is untouched by the + refusal path.""" + d = write_case(tmp_path, meta=valid_meta()) + assert run_verify(tmp_path, d) == 0 + out, err = capsys.readouterr() + assert "level: VERIFIED" in out + assert "Traceback" not in out + err + + +def test_a_wrong_implementation_is_still_rejected(tmp_path, capsys): + """**Passes on the unfixed tree too** — a pin. rc 1 must keep meaning "the code + failed verification": if bad input also returned 1, this test could not tell them + apart from a caller's side.""" + d = write_case(tmp_path, meta=valid_meta(), impl=WRONG) + assert run_verify(tmp_path, d) == 1 + out, err = capsys.readouterr() + assert "level: REJECTED" in out + assert "Traceback" not in out + err From a28347fa6c9fb4c709e62b685640819421d2b89b Mon Sep 17 00:00:00 2001 From: jitagent Dev-1 Date: Thu, 5 Nov 2026 01:40:52 +0000 Subject: [PATCH 2/2] Catch a case file nested too deeply to parse json.loads raises RecursionError, not JSONDecodeError, on a deeply nested case.json, so a 10,000-deep file still came back as rc 1 with a traceback - indistinguishable from a REJECTED verdict. Caught in the same place as the other unreadable-file branches, with no invented depth number: the threshold is stack-dependent. Found in Security's review of this branch; this repo's #16/#18 are deep-nesting reports, so a deep case file is a realistic input. --- CHANGELOG.md | 5 +++-- src/jitagent/cases.py | 2 ++ tests/test_verify_case_errors.py | 12 ++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42dffb6..ffb927a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,9 @@ is 0, the API may change in a minor release. `case.json`, invalid JSON, a top level that is not an object, a missing `spec`/`examples`/`expect` (or `expect.level`), an unknown key in `spec` or an example, an `examples` value that is not a list, an example that is not an object or - lacks `input`/`output`, a missing implementation file, and a file that is not UTF-8 - (the classic Windows save). Valid cases render exactly as before. + lacks `input`/`output`, a missing implementation file, a file that is not UTF-8 + (the classic Windows save), and a file nested too deeply to parse. Valid cases render + exactly as before. ## [0.0.1] — 2026-09-16 diff --git a/src/jitagent/cases.py b/src/jitagent/cases.py index bbc0287..8839c33 100644 --- a/src/jitagent/cases.py +++ b/src/jitagent/cases.py @@ -68,6 +68,8 @@ def load(cls, d: Path) -> "Case": meta = json.loads(raw) except json.JSONDecodeError as e: raise CaseError(f"{str(meta_path)!r} is not valid JSON: {e}") from None + except RecursionError: + raise CaseError(f"{str(meta_path)!r} nests too deeply to parse") from None if not isinstance(meta, dict): kind = _kind(meta) raise CaseError( diff --git a/tests/test_verify_case_errors.py b/tests/test_verify_case_errors.py index 8fd9d59..0806b95 100644 --- a/tests/test_verify_case_errors.py +++ b/tests/test_verify_case_errors.py @@ -94,6 +94,18 @@ def test_a_case_file_that_is_not_utf8_is_a_usage_error(tmp_path, capsys): assert "cannot read" in line +def test_a_case_file_nested_too_deeply_is_a_usage_error(tmp_path, capsys): + """Past the stack limit `json.loads` raises `RecursionError`, not `JSONDecodeError`, + so it is a third kind of unreadable file. A case file is exactly where deep nesting + arrives — this repo's #16/#18 are reports about deep values, and whoever reproduces + them writes one. 50,000 levels is far past any stack and cheap to write as text; + dumping a deep object here would test the writer instead of the loader.""" + d = write_case(tmp_path) + (d / "case.json").write_text("[" * 50_000 + "]" * 50_000) + line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json", + "nests too deeply to parse") + + def test_invalid_json_names_the_line_and_column(tmp_path, capsys): d = write_case(tmp_path, text='{"name": "add",\n "spec": {},') line = assert_refused(capsys, run_verify(tmp_path, d), d / "case.json",