overseer: advisor parser handles bare-object JSON in prose (#2116)

shared/egg_overseer/advisor.py

@@ -234,10 +234,34 @@ async def _default_runner(p: str, m: str) -> str:
 
     try:
         payload = json.loads(cleaned)
-    except json.JSONDecodeError as exc:
-        raise AdvisorParseError(
-            f"consult_advisor: SDK response is not valid JSON: {raw!r}"
-        ) from exc
+    except json.JSONDecodeError:
+        # Fall-back: pull the first balanced JSON object out of prose.
+        # Under ``max_turns=1`` Claude often emits ``Here's the
+        # verdict:\n{...}`` instead of pure JSON; ``raw_decode`` is
+        # string-aware so braces inside JSON string values don't fool
+        # the scan. Iterate over candidate ``{`` positions so a stray
+        # brace in leading prose (e.g. ``see {field} below: {...}``)
+        # doesn't lock us onto an unparseable snippet.
+        decoder = json.JSONDecoder()
+        payload = None
+        last_exc: json.JSONDecodeError | None = None
+        pos = 0
+        while True:
+            start = cleaned.find("{", pos)
+            if start == -1:
+                break
+            try:
+                payload, _ = decoder.raw_decode(cleaned[start:])
+                break
+            except json.JSONDecodeError as exc:
+                last_exc = exc
+                pos = start + 1
+        if payload is None:
+            # No `{` ever found → no useful inner exception; otherwise
+            # chain the most recent ``raw_decode`` failure for context.
+            raise AdvisorParseError(
+                f"consult_advisor: SDK response is not valid JSON: {raw!r}"
+            ) from last_exc
 
     try:
         verdict = AdvisorVerdict.model_validate(payload)

shared/tests/test_overseer_advisor.py

@@ -193,6 +193,139 @@ def test_invalid_json_raises_parse_error(self) -> None:
                 )
             )
 
+    def test_bare_object_only_payload(self) -> None:
+        # A pure bare JSON object — the most common well-behaved shape.
+        runner = self._runner_returning('{"decision": "watch", "reasoning": "r"}')
+        verdict = asyncio.run(
+            consult_advisor(
+                classification={},
+                health_alerts=[],
+                progress_events=[],
+                recent_log_lines=[],
+                _agent_runner=runner,
+            )
+        )
+        assert verdict.decision == "watch"
+
+    def test_fenced_block_payload(self) -> None:
+        # ```json … ``` wrapper — defensive fence-strip path.
+        runner = self._runner_returning('```json\n{"decision": "watch", "reasoning": "r"}\n```')
+        verdict = asyncio.run(
+            consult_advisor(
+                classification={},
+                health_alerts=[],
+                progress_events=[],
+                recent_log_lines=[],
+                _agent_runner=runner,
+            )
+        )
+        assert verdict.decision == "watch"
+
+    def test_prose_with_bare_object_payload(self) -> None:
+        # Prose-then-bare-JSON: the carry-forward case from #2096 review.
+        runner = self._runner_returning(
+            'Here is my verdict:\n{"decision": "watch", "reasoning": "r"}\n'
+        )
+        verdict = asyncio.run(
+            consult_advisor(
+                classification={},
+                health_alerts=[],
+                progress_events=[],
+                recent_log_lines=[],
+                _agent_runner=runner,
+            )
+        )
+        assert verdict.decision == "watch"
+
+    def test_prose_around_fenced_payload(self) -> None:
+        # Fence in the middle of prose (startswith check misses it,
+        # raw_decode fall-back catches it).
+        runner = self._runner_returning(
+            "Looking at the signals:\n"
+            '```json\n{"decision": "watch", "reasoning": "r"}\n```\n'
+            "That is my call."
+        )
+        verdict = asyncio.run(
+            consult_advisor(
+                classification={},
+                health_alerts=[],
+                progress_events=[],
+                recent_log_lines=[],
+                _agent_runner=runner,
+            )
+        )
+        assert verdict.decision == "watch"
+
+    def test_no_brace_raises_parse_error(self) -> None:
+        # Confirms the fall-back doesn't paper over genuinely empty
+        # responses — error semantics for unparseable text stay intact.
+        runner = self._runner_returning("absolutely no json here")
+        with pytest.raises(AdvisorParseError, match="not valid JSON"):
+            asyncio.run(
+                consult_advisor(
+                    classification={},
+                    health_alerts=[],
+                    progress_events=[],
+                    recent_log_lines=[],
+                    _agent_runner=runner,
+                )
+            )
+
+    def test_embedded_brace_in_string_value(self) -> None:
+        # Locks the string-aware contract: ``raw_decode`` must stop at
+        # the closing ``}`` of the JSON object even when value strings
+        # contain a literal ``}``. A naive brace-counting scan would
+        # truncate the payload and break validation.
+        runner = self._runner_returning(
+            'Verdict:\n{"decision": "watch", "reasoning": "value with } embedded brace"}'
+        )
+        verdict = asyncio.run(
+            consult_advisor(
+                classification={},
+                health_alerts=[],
+                progress_events=[],
+                recent_log_lines=[],
+                _agent_runner=runner,
+            )
+        )
+        assert verdict.decision == "watch"
+        assert verdict.reasoning == "value with } embedded brace"
+
+    def test_trailing_prose_after_payload(self) -> None:
+        # ``raw_decode`` accepts trailing data after a complete JSON
+        # value, so unfenced prose tacked on the end of a verdict is
+        # tolerated even though plain ``json.loads`` rejects it with
+        # "Extra data".
+        runner = self._runner_returning('{"decision": "watch", "reasoning": "r"} thanks!')
+        verdict = asyncio.run(
+            consult_advisor(
+                classification={},
+                health_alerts=[],
+                progress_events=[],
+                recent_log_lines=[],
+                _agent_runner=runner,
+            )
+        )
+        assert verdict.decision == "watch"
+
+    def test_stray_leading_brace_skipped(self) -> None:
+        # Leading prose contains a ``{`` that is NOT the start of the
+        # verdict (e.g. a templated placeholder). The loop must skip
+        # past the unparseable snippet and find the real JSON object.
+        runner = self._runner_returning(
+            'see {field_name} below: {"decision": "watch", "reasoning": "r"}'
+        )
+        verdict = asyncio.run(
+            consult_advisor(
+                classification={},
+                health_alerts=[],
+                progress_events=[],
+                recent_log_lines=[],
+                _agent_runner=runner,
+            )
+        )
+        assert verdict.decision == "watch"
+
     def test_schema_failure_raises_parse_error(self) -> None:
         runner = self._runner_returning({"decision": "alert"})  # missing reasoning
         with pytest.raises(AdvisorParseError, match="validation"):