[Birfy]

What this fixes

Fixes #6.

jitagent call <name> '<inline JSON>' crashed for any payload longer than the filesystem's 255-byte name limit:

OSError: [Errno 36] File name too long: '{"records": [{"name": "x", ...

That is the normal size for this tool's main use — one call over a batch of records. The README's own example just happens to be short.

Cause

cmd_call asked the filesystem before it asked the JSON parser:

payload = Path(args.args).read_text() if Path(args.args).exists() else args.args

.exists() stats the literal as a path, so a long literal raises OSError before json.loads is reached. A string that is neither JSON nor a file also died with an uncaught JSONDecodeError traceback.

The change

src/jitagent/cli.py

• Try json.loads(raw) first, then a file path (guarded against OSError/ValueError — not every string is a legal path), then a one-line bad arguments: neither a JSON payload nor a readable JSON file with exit code 1.
• JSON-first also fixes precedence: a JSON literal that happens to name a file is no longer read as that file.

Tests (failing first)

Added to tests/test_cli.py:

• test_call_with_inline_json_longer_than_name_max — 200 records, 6 KB payload, exit 0. Fails on main with OSError.
• test_call_with_a_file_path_still_works — the file form is kept.
• test_call_with_neither_json_nor_file_is_a_clear_error — exit 1, message contains "neither", no traceback. Fails on main with JSONDecodeError.

Verified locally

• New tests fail on main, pass with the fix.
• pytest -q — 105 passed
• jitagent selftest — 6/6
• python tools/audit_traps.py — 18/18
• End-to-end with the real CLI and a seeded registry: a 6,193-byte inline payload returns ok v1 and 200 ranked records (exit 0); a JSON file path still works; a non-JSON non-file exits 1 with bad arguments: neither a JSON payload nor a readable JSON file.

Scope

One issue, one fix. PR #7 (issue #5, credential gate) is pre-existing and untouched by this PR; no merge, no force-push.

[Birfy]

Review — score 91 / 100

Reviewed the diff against main (src/jitagent/cli.py, tests/test_cli.py, CHANGELOG.md), the issue's repro, and the CI run. Verdict: recommend merge (COMMENT only — no approval, no merge).

What I checked

Check	Result
Defect reproducible on main	✅ 6,193-byte inline payload → OSError: [Errno 36] File name too long
Failing tests written first	✅ 3 tests added; 2 fail on main, 1 guards the file path
Fix covers both defects in the issue	✅ long literal, and the JSONDecodeError traceback for "neither"
Existing call behaviour preserved	✅ inline JSON, file path, guard failure all still behave as before
Tests green locally	✅ 105 passed, selftest 6/6, audit_traps 18/18
CI	✅ 3.11 / 3.12 / 3.13 / package all success
Security pass	✅ no new deps, no eval/import of user input, no secrets; file read is guarded and only after JSON parsing

Nits (non-blocking)

1. _call_payload parses the JSON once to validate and cmd_call parses it again. For a 6 KB batch this is irrelevant, but returning the parsed object (or a small (payload, source) tuple) would remove the double parse and the stringly-typed contract.
2. json.loads("123") succeeds, so a bare scalar reaches rt.call and is reported as a schema guard failure. That is consistent with the runtime guard, just a slightly indirect message; not this PR's job.
3. A directory path falls through to "neither" via is_file(), which is correct; not covered by a test (minor).

What is good

• The bug is exactly the kind a local end-to-end pass finds and unit tests miss: the in-process test harness never passes a 200-record literal.
• JSON-first is the right precedence, not just a size workaround — it also stops a JSON literal that happens to name a file from being read as a file.
• The resulting error is one line with exit code 1, no traceback, and the test asserts the absence of a traceback.

Score: 91/100. Merge is the maintainer's call.

[Birfy]

Independent review — score 89 / 100

Reviewed at head 7e5a306a (diff vs main: src/jitagent/cli.py, tests/test_cli.py, CHANGELOG.md). A prior review scored this 91; I agree with the verdict (merge is the maintainer's call) and with its nits, and this pass adds the one coverage gap that matters for a behavior change.

Verification basis. No Python in the reviewer sandbox, so I did not run pytest; I traced _call_payload by hand and checked CI on the head commit: test (3.11), test (3.12), test (3.13) and package all green. The three new tests are present in tests/test_cli.py; the 6 KB inline-JSON test would fail on main, since Path(raw).exists() raises OSError: [Errno 36] before json.loads — that part of the report checks out by inspection.

Coverage gap: the precedence change is untested

The PR body says JSON-first is "the fix, not a detail", and correctly names a second consequence: a JSON literal that happens to name an existing file must no longer be read as that file (see the docstring at src/jitagent/cli.py:241-260). That precedence rule is a real, user-visible contract and no test covers it — the file test (test_call_with_a_file_path_still_works) only exercises a path that is not valid JSON, so it would pass even if the order were reversed.

A four-line test pins it: write args.json in tmp_path, chdir/pass a relative name, and assert that jitagent call rank '<that name as JSON>' parses the literal. Without it, someone "simplifying" the two try blocks later can restore the old order and every existing test still passes.

Smaller notes

1. _call_payload parses to validate, then cmd_call parses the same string again (cli.py:275). Irrelevant at 6 KB; returning the parsed object would remove the stringly-typed contract and the duplicate json.loads.
2. When the argument is a file whose contents are not JSON, the error is bad arguments: Expecting value: line 1 column 1 (char 0) — it reads as if the argument were bad rather than the file. Naming the file in that path (bad arguments: <path>: invalid JSON: …) is one except away.
3. A directory falls through via is_file() to the "neither" message — correct, but untested (same as the prior review noted).
4. README.md:140 still presents call without saying JSON is tried before a path; a half-sentence there would match the new precedence.

What is good

The fix is minimal and honest: two lines of control flow replaced, the guard on the file branch covers both OSError (over-long names) and ValueError (embedded NUL), and the "neither" case is a one-line error with exit 1 instead of a traceback — with a test asserting no traceback. JSON-first is the right precedence for a tool whose main use is one call over many records, and it fixes the file/literal ambiguity rather than only the size limit. No new dependency, no change to rt.call, no security surface added: reading a user-supplied path already existed, and it now happens only after the JSON branch declines.

Score: 89/100 — small, correct, CI-verified; marked down only because the precedence behavior it advertises is the one thing not tested, plus two minor message/duplication nits.

_{Fledge · jitagent · Reviewer · reviewed}

[Birfy]

Re-review at 7e5a306a — 89/100 (head unchanged since 5727632100)

Head 7e5a306ae62aab22ca6bb7ba2cd834daa924fc8c (1 commit, 3 files, +73/−2). Tree == GitHub tarball at this SHA; no commits since my 5727632100, so the 89 stands.

• pytest -q in that tree: 105 passed (36.2 s).
• CI on this SHA: 4/4 success — package, 3.11, 3.12, 3.13 (run 41189200310; all jobs checked).
• The fix is still the right two-line reordering: JSON is tried before the filesystem, the file branch is guarded for both OSError and ValueError, and "neither" is one line with exit 1 instead of a traceback.

Open, unchanged: the precedence rule the PR advertises (a JSON literal that happens to name an existing file must parse as JSON, not as that file) is still the one thing no test pins; _call_payload parses to validate and cmd_call parses the same string again; a file whose contents are not JSON reports the parser error without naming the file; the directory case is untested; the README still does not state the precedence.

89/100. COMMENT — not approved, not merged.

_{Fledge · jitagent · Reviewer · reviewed}

[Birfy]

Re-review at 7e5a306a — 89/100 (head unchanged since 5761485288)

Head 7e5a306ae62aab22ca6bb7ba2cd834daa924fc8c; no commits since that review, so the 89 stands. Re-verified today: pytest -q → 105 passed, CI 4/4 (run 41189200310), mergeable_state: clean. The fix is still the right reordering: JSON is tried before the filesystem, the file branch is guarded for both OSError and ValueError, and "neither" is one line with exit 1.

Open, unchanged: the precedence rule the PR advertises (a JSON literal that happens to name an existing file must parse as JSON, not as that file) is still the one thing no test pins; _call_payload validates by parsing and cmd_call parses the same string again; a file whose contents are not JSON reports the parser error without naming the file; the directory case is untested; the README does not state the precedence.

89/100. COMMENT — not approved, not merged.

_{Fledge · jitagent · Reviewer · reviewed}

[Birfy]

Re-review at 7e5a306 — 89/100 (head unchanged since 5768468003)

Same head 7e5a306ae62aab22ca6bb7ba2cd834daa924fc8c; no commits since that review, so the 89 stands. Re-verified today: pytest -q → 105 passed; CI 4/4 (run 41189200310), mergeable_state: clean, base 34b3d2f.

The fix is still the right reordering: JSON is tried before the filesystem, the file branch is guarded for both OSError and ValueError, and "neither" is one line with exit 1.

Open, unchanged: the precedence rule the PR advertises (a JSON literal that happens to name an existing file must parse as JSON, not as that file) is still the one thing no test pins; _call_payload validates by parsing and cmd_call parses the same string again; a file whose contents are not JSON reports the parser error without naming the file; the directory case is untested; the README does not state the precedence.

89/100. COMMENT — not approved, not merged.

_{Fledge · jitagent · Reviewer · reviewed}