From 89500ad6a08679a540f38a62c2e3bb079a42a953 Mon Sep 17 00:00:00 2001 From: Pravit Ampapathini Date: Sat, 15 Aug 2026 23:57:04 -0700 Subject: [PATCH] fix(sleep): skip assistant.message events with non-dict data CopilotCliBackend._parse_jsonl_response assumed the data field of an assistant.message event was an object, so a truthy non-dict value raised AttributeError from the field access. That escaped the per-line try, which only wraps json.loads, and killed the parse of the entire stream. Port the isinstance guard already used by parse_copilot_jsonl in skillopt/model/copilot_backend.py, which was hardened in 5497a31 but did not reach this vendored copy. The wider except clause is kept, since json.loads raises RecursionError rather than JSONDecodeError on deeply nested payloads. Fixes the pre-existing failure in tests/test_sleep_engine.py::TestCopilotBackend::test_parse_jsonl_ignores_excessively_nested_json Co-Authored-By: Claude Opus 5 --- skillopt_sleep/backend.py | 25 ++++++++++++++++++++++++- tests/test_sleep_engine.py | 20 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/skillopt_sleep/backend.py b/skillopt_sleep/backend.py index 95b44b23..c331a721 100644 --- a/skillopt_sleep/backend.py +++ b/skillopt_sleep/backend.py @@ -1570,6 +1570,24 @@ def _call(self, prompt: str, *, max_tokens: int = 1024) -> str: @staticmethod def _parse_jsonl_response(raw: str) -> str: + """Concatenate assistant text from a Copilot JSONL event stream. + + Behaviourally identical to ``skillopt.model.copilot_backend``'s + ``parse_copilot_jsonl``; vendored because this package keeps ZERO + dependency on the research package (see the module docstring of + ``skillopt_sleep.gate`` for the same arrangement). Keep the two in sync. + + A malformed event must never take down the whole stream: the CLI emits + one JSON object per line, so a single bad line loses at most that line's + text while the surrounding ``assistant.message`` events still parse. + ``data`` is therefore type-checked rather than assumed to be an object. + A truthy non-dict (``"text"``, ``5``, a non-empty list) would otherwise + raise ``AttributeError`` from the field access, which no caller catches. + + The exception list is deliberately wider than the research-package copy: + ``json.loads`` raises ``RecursionError`` rather than ``JSONDecodeError`` + on a deeply nested payload. + """ parts: List[str] = [] for line in raw.splitlines(): line = line.strip() @@ -1579,8 +1597,13 @@ def _parse_jsonl_response(raw: str) -> str: obj = json.loads(line) except (ValueError, RecursionError, TypeError): continue + if not isinstance(obj, dict): + continue if obj.get("type") == "assistant.message": - content = (obj.get("data") or {}).get("content") + data = obj.get("data") + if not isinstance(data, dict): + continue + content = data.get("content") if isinstance(content, str) and content: parts.append(content) return "\n".join(parts).strip() diff --git a/tests/test_sleep_engine.py b/tests/test_sleep_engine.py index cfd05162..d3113865 100644 --- a/tests/test_sleep_engine.py +++ b/tests/test_sleep_engine.py @@ -1435,6 +1435,26 @@ def test_parse_jsonl_ignores_oversized_integer(self): raw = '{"type":"assistant.message","data":' + "9" * 5000 + "}" self.assertEqual(CopilotCliBackend._parse_jsonl_response(raw), "") + def test_parse_jsonl_skips_non_dict_data_without_losing_stream(self): + # A malformed event must cost only its own line: the surrounding + # assistant.message events still have to reach the caller. Mirrors + # tests/test_copilot_exec_backend.py for the research-package copy. + from skillopt_sleep.backend import CopilotCliBackend + for bad in ('"text"', "5", "[1,2]", "true"): + raw = "\n".join([ + '{"type":"assistant.message","data":{"content":"first"}}', + '{"type":"assistant.message","data":' + bad + "}", + '{"type":"assistant.message","data":{"content":"second"}}', + ]) + with self.subTest(data=bad): + self.assertEqual( + CopilotCliBackend._parse_jsonl_response(raw), "first\nsecond" + ) + + def test_parse_jsonl_ignores_non_object_top_level(self): + from skillopt_sleep.backend import CopilotCliBackend + self.assertEqual(CopilotCliBackend._parse_jsonl_response("[]\nnull\n"), "") + def test_isolated_home_by_default(self): from skillopt_sleep.backend import CopilotCliBackend be = CopilotCliBackend()