diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a61708..829b34e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -525,6 +525,15 @@ Notable changes to Cloudfall. The format follows Before, a run against unreachable hosts crashed while writing the `failed` receipt, left the proposal `proposed` with no trace that it ran, and showed the caller a traceback +- The investigation record of `agent investigate` bounds the rest of the + model's text. The finding's `rootCause` and `why` keep at most 8000 + characters, keeping the start before a `[last N characters cut]` mark; + the proposed decision ids are still read from the whole line. A tool call + whose arguments serialize to more than 8000 characters of JSON is refused + before it runs and recorded as an `invalid` step with empty arguments, so + `spec.steps[].arguments` only holds arguments that ran whole; the trace + still keeps the start of the raw arguments. `agent-investigation.schema.json` + bounds both finding fields with `maxLength` (#63) ### Security diff --git a/config/schemas/v1/agent-investigation.schema.json b/config/schemas/v1/agent-investigation.schema.json index 9c65a55..0cc2003 100644 --- a/config/schemas/v1/agent-investigation.schema.json +++ b/config/schemas/v1/agent-investigation.schema.json @@ -91,7 +91,8 @@ "maxLength": 128 }, "arguments": { - "type": "object" + "type": "object", + "description": "The call's parsed arguments. A call whose arguments serialize to more than 8000 characters is refused before it runs and recorded as invalid with empty arguments." }, "outcome": { "enum": [ @@ -179,7 +180,9 @@ "type": "object", "properties": { "rootCause": { - "type": "string" + "type": "string", + "maxLength": 8000, + "description": "The answer's root cause line; a longer one keeps its start, before a mark naming how many characters were cut." }, "proposed": { "type": "array", @@ -194,7 +197,9 @@ } }, "why": { - "type": "string" + "type": "string", + "maxLength": 8000, + "description": "The answer's why line; a longer one keeps its start, before a mark naming how many characters were cut." } }, "required": [ diff --git a/sdk/src/cloudfall/investigation.py b/sdk/src/cloudfall/investigation.py index 6732196..f521dfc 100644 --- a/sdk/src/cloudfall/investigation.py +++ b/sdk/src/cloudfall/investigation.py @@ -44,6 +44,10 @@ """Most characters of the model's answer the record keeps, cut mark included.""" ARGUMENTS_LIMIT = TOOL_TEXT_LIMIT """Most characters of a tool call's raw arguments the trace keeps, cut mark included.""" +FINDING_TEXT_LIMIT = TOOL_TEXT_LIMIT +"""Most characters of a finding's root cause or why the record keeps, mark included.""" +STEP_ARGUMENTS_LIMIT = TOOL_TEXT_LIMIT +"""Most characters a call's parsed arguments serialize to before it is refused unrun.""" TOOL_NAME_LIMIT = 128 """Most characters of a tool name the record keeps, as its schema allows.""" REPLY_ID_LIMIT = 256 @@ -585,9 +589,9 @@ def _trace_text(text: str) -> str: def _head_text(text: str, limit: int) -> str: - """Keep the start of a long answer or raw arguments, where they begin, and say so. + """Keep the start of a long answer, finding line or raw arguments, and say so. - Both are the model's text and can quote host output, so the record + All are the model's text and can quote host output, so the record bounds them as it bounds the trace's reasoning, but keeps the start: an answer opens with its finding. The mark counts against the limit: what is kept fits the schema's ``maxLength``. @@ -699,10 +703,11 @@ def _run_call( try: arguments = json.loads(raw) if isinstance(raw, str) else raw except json.JSONDecodeError as error: - result = _refusal(StepOutcome.INVALID, f"arguments are not JSON: {error}") - return Step(recorded_name, {}, result.outcome, None), result - if not isinstance(arguments, Mapping): - result = _refusal(StepOutcome.INVALID, "arguments must be a JSON object") + problem: str | None = f"arguments are not JSON: {error}" + else: + problem = _arguments_problem(arguments) + if problem is not None: + result = _refusal(StepOutcome.INVALID, problem) return Step(recorded_name, {}, result.outcome, None), result arguments = cast("Mapping[str, object]", arguments) tool = tools.get(name) @@ -739,6 +744,20 @@ def _run_call( return Step(name, arguments, result.outcome, result.decision), result +def _arguments_problem(arguments: object) -> str | None: + if not isinstance(arguments, Mapping): + return "arguments must be a JSON object" + # Key order as parsed and no ASCII escapes give one length per call, in + # characters; a step keeps only arguments that ran whole, so a longer + # call is refused before it runs. + if len(json.dumps(arguments, ensure_ascii=False)) > STEP_ARGUMENTS_LIMIT: + return ( + f"arguments are longer than {STEP_ARGUMENTS_LIMIT} characters as JSON; " + "nothing ran" + ) + return None + + def _refusal(outcome: StepOutcome, message: str) -> ToolResult: return ToolResult(outcome=outcome, content={"ok": False, "error": message}) @@ -764,11 +783,13 @@ def _finding(answer: str, steps: Sequence[Step]) -> Finding | None: } written = {step.decision for step in steps if step.decision is not None} named = list(dict.fromkeys(_DECISION_ID.findall(fields["proposed"]))) + # The ids are read from the whole proposed line; the record keeps the + # start of the two free-text lines. return Finding( - root_cause=fields["root cause"], + root_cause=_head_text(fields["root cause"], FINDING_TEXT_LIMIT), proposed=tuple(entry for entry in named if entry in recorded), unrecorded=tuple(entry for entry in named if entry not in written), - why=fields["why"], + why=_head_text(fields["why"], FINDING_TEXT_LIMIT), ) diff --git a/sdk/tests/test_investigation.py b/sdk/tests/test_investigation.py index 5e3cf4e..d62061a 100644 --- a/sdk/tests/test_investigation.py +++ b/sdk/tests/test_investigation.py @@ -19,6 +19,8 @@ ARGUMENTS_LIMIT, ERROR_INVESTIGATIONS_UNWRITABLE, ERROR_MODEL_REFUSED, + FINDING_TEXT_LIMIT, + STEP_ARGUMENTS_LIMIT, TRACE_TEXT_LIMIT, AgentTool, InvestigationError, @@ -1172,6 +1174,67 @@ def test_a_finding_past_the_cut_is_still_read_from_the_whole_answer() -> None: assert "ROOT CAUSE" not in done.answer +@pytest.mark.parametrize( + "length", [FINDING_TEXT_LIMIT, FINDING_TEXT_LIMIT + 1, 50_000] +) +def test_long_finding_lines_keep_their_start(tmp_path: Path, length: int) -> None: + report, rotate = _tools() + # Characters, not bytes: the Cyrillic line is counted the same. + cause = ("диск-" * length)[:length] + why = ("why-" * length)[:length] + answer = f"ROOT CAUSE: {cause}\nPROPOSED: none\nWHY: {why}\n" + + done = investigate( + "alert", _tool_list(report, rotate), ENDPOINT, _Script(_say(answer)) + ) + saved = InvestigationStore( + directory=tmp_path / "investigations", + catalog=SchemaCatalog(default_schema_directory()), + ).save(done) + + assert FINDING_TEXT_LIMIT == 8000 + assert saved.finding is not None + for whole, kept in [(cause, saved.finding.root_cause), (why, saved.finding.why)]: + if length == FINDING_TEXT_LIMIT: + assert kept == whole + continue + assert FINDING_TEXT_LIMIT - 3 <= len(kept) <= FINDING_TEXT_LIMIT + head, mark = kept.rsplit("\n", 1) + assert whole.startswith(head) + assert mark == f"[last {len(whole) - len(head)} characters cut]" + + +def test_the_proposed_line_is_read_whole_past_the_finding_limit() -> None: + report, rotate = _tools() + decision = "shop-logrotate-20261008152628" + proposed = "x" * 9000 + f" {decision}" + answer = f"ROOT CAUSE: disk\nPROPOSED: {proposed}\nWHY: rotate.\n" + script = _Script(_call("shop-logrotate", {}), _say(answer)) + + done = investigate("alert", _tool_list(report, rotate), ENDPOINT, script) + + assert done.finding is not None + assert done.finding.proposed == (decision,) + + +@pytest.mark.parametrize("field", ["rootCause", "why"]) +def test_the_schema_refuses_a_finding_line_past_the_limit(field: str) -> None: + report, rotate = _tools() + done = investigate( + "alert", _tool_list(report, rotate), ENDPOINT, _Script(_say(_FINDING)) + ) + document = done.as_document() + spec = document["spec"] + assert isinstance(spec, dict) + catalog = SchemaCatalog(default_schema_directory()) + spec["finding"][field] = "x" * FINDING_TEXT_LIMIT + catalog.validate_named("agent-investigation.schema.json", document) + spec["finding"][field] = "x" * (FINDING_TEXT_LIMIT + 1) + + with pytest.raises(ValidationError): + catalog.validate_named("agent-investigation.schema.json", document) + + def test_the_cli_streams_the_answer_as_the_record_keeps_it( tmp_path: Path, capsys: pytest.CaptureFixture[str], model_url: str ) -> None: @@ -1212,7 +1275,7 @@ def _call(self, arguments: Mapping[str, object]) -> ToolResult: @pytest.mark.parametrize("length", [ARGUMENTS_LIMIT, ARGUMENTS_LIMIT + 1, 50_000]) -def test_long_raw_arguments_keep_their_start_in_the_trace_only( +def test_long_arguments_are_refused_unrun_and_cut_in_the_trace( tmp_path: Path, length: int ) -> None: echo = _Echo() @@ -1228,20 +1291,68 @@ def test_long_raw_arguments_keep_their_start_in_the_trace_only( catalog=SchemaCatalog(default_schema_directory()), ).save(done) - # The tool ran on the whole arguments; only the trace's copy is cut. - assert echo.received == [{"note": note}] - assert saved.steps[0].arguments == {"note": note} kept = saved.trace[0].tool_calls[0].arguments - assert ARGUMENTS_LIMIT == 8000 + assert ARGUMENTS_LIMIT == STEP_ARGUMENTS_LIMIT == 8000 if length == ARGUMENTS_LIMIT: + # At the limit the tool runs on the whole arguments, kept whole. + assert echo.received == [{"note": note}] + assert saved.steps[0].arguments == {"note": note} + assert saved.steps[0].outcome is StepOutcome.RAN assert kept == raw return + # Past it nothing runs and the step keeps no arguments (#63) ... + assert echo.received == [] + assert saved.steps[0].arguments == {} + assert saved.steps[0].outcome is StepOutcome.INVALID + told = script.requests[1]["messages"] + assert isinstance(told, list) + assert "longer than 8000 characters" in told[-1]["content"] + # ... while the trace still keeps the start of the model's text (#57). assert ARGUMENTS_LIMIT - 3 <= len(kept) <= ARGUMENTS_LIMIT head, mark = kept.rsplit("\n", 1) assert raw.startswith(head) assert mark == f"[last {length - len(head)} characters cut]" +@pytest.mark.parametrize( + ("length", "ran"), + [(STEP_ARGUMENTS_LIMIT - 1, True), (STEP_ARGUMENTS_LIMIT + 1, False)], +) +def test_non_ascii_arguments_are_measured_in_characters( + length: int, *, ran: bool +) -> None: + echo = _Echo() + # Characters, not ASCII escapes: a Cyrillic note just under the limit runs. + note = "ж" * (length - len(json.dumps({"note": ""}))) + assert len(json.dumps({"note": note}, ensure_ascii=False)) == length + script = _Script(_call("note", {"note": note}), _say(_FINDING)) + + done = investigate("alert", [echo.tool], ENDPOINT, script) + + if ran: + assert echo.received == [{"note": note}] + assert done.steps[0].outcome is StepOutcome.RAN + else: + assert echo.received == [] + assert done.steps[0].outcome is StepOutcome.INVALID + + +def test_a_refused_long_call_is_not_remembered_as_run() -> None: + echo = _Echo() + note = "n" * STEP_ARGUMENTS_LIMIT + script = _Script( + _call("note", {"note": note}, "c1"), + _call("note", {"note": note}, "c2"), + _say(_FINDING), + ) + + done = investigate("alert", [echo.tool], ENDPOINT, script) + + # The second call is refused again, not served as a repeat of a run. + assert echo.received == [] + assert [step.outcome for step in done.steps] == [StepOutcome.INVALID] * 2 + + @pytest.mark.parametrize("field", ["answer", "arguments"]) def test_the_schema_refuses_an_answer_or_arguments_past_the_limit( field: str,