Repository navigation
fix(v2): detect multiline JSON-RPC messages - #4
Conversation
| ) | ||
| combined = bytearray(entry.raw) | ||
| for following in received[index + 1 :]: | ||
| if following.parse_error is None: |
There was a problem hiding this comment.
Problem: the scan stops at the first following line that parses as JSON on its own, and it has no other stop. That gives a false PASS in one case and a very slow run in another.
How it happens:
- An agent pretty-prints a message that has an array of strings, e.g.
"tags": ["a", "b"]withindent=2. - The line
"b"is valid JSON by itself, sofollowing.parse_error is Nonehitsbreakbefore the closing lines are joined. ACP-TRANSPORT-203PASSes for a message split over many lines. I ran such an agent: 201 FAIL, 203 PASS.
Second case: when the run of unparsable lines is long (an agent that logs to stdout), every start index rescans the rest of the run and decodes a growing buffer, so the cost is about n^3. With a conforming agent that prints log lines after EOF, the whole run took 1.5s for 1000 lines and 21s for 3000 lines. On main it takes 1.3s for 3000 lines.
Impact: a framing violation can go unreported, and noisy agents can hit --test-timeout. Both come from this PR's loop.
Fix: do not stop on a valid line. Stop when the joined text is already malformed before its end, because more lines cannot repair it:
for following in received[index + 1 :]:
combined.extend(b"\n")
combined.extend(following.raw)
try:
text = combined.decode("utf-8")
json.loads(text)
except UnicodeDecodeError:
break
except json.JSONDecodeError as exc:
if exc.pos < len(text.rstrip()):
break # malformed before the end; more lines cannot fix it
continue
pytest.fail(...)I applied this temporarily: the string-array agent now FAILs 203, the PR's fixture still FAILs, the 3000-line agent PASSes in 0.13s, garbage_after_response.py and conforming.py PASS, and the related tests/v2/test_cli.py tests pass. The new fixture has no array, so it would not catch the first case; adding a string array to its message (e.g. in _meta) would.
| f"{entry.raw!r} + {received[index + 1].raw!r}" | ||
| ) | ||
| combined = bytearray(entry.raw) | ||
| for following in received[index + 1 :]: |
There was a problem hiding this comment.
The docstring above (lines 84-88) still says the reassembly is "the next entry's raw bytes". It now joins any number of following lines. Please update it.
|
Addressed on the current head ( Validation completed here: |
Summary
Validation
uv run pytest tests/v2/test_cli.py -k 'transport or message_split_across_multiple_lines' -quv run --with ruff ruff check src/tck/v2/conformance/test_transport.py tests/fixtures/agents/v2/message_split_across_multiple_lines.pygit diff --check