From a66699d0a8c26e164fa5a19f09903123c47cb6fd Mon Sep 17 00:00:00 2001 From: VitjanZ Date: Mon, 14 Sep 2026 11:26:45 +0200 Subject: [PATCH 1/5] Fix unit tests with a server-side agent session Replace the stateless /fix_unittests_issue call with an agent loop: the first failure starts a session via /agent/start, the client executes the tool calls the model asks for (read/grep/ls/edit/write/delete files, run the unit tests) and posts results to /agent/continue until the agent calls submit_fix. The state machine then re-runs the unit tests as before; a repeated failure is fed back into the same session as the submit_fix result, so earlier attempts stay in context instead of being re-diagnosed from scratch. - render_machine/agent_tools.py: tool implementations confined to the build folder (writes) and project root (reads); changed files are tracked in the unit tests running context so they reach the FRID commit as before. - FixUnitTests: start/continue the session, per-attempt turn cap, fresh session when the previous one ended without a submission. - UnitTestsRunningContext carries the session id and the pending submit_fix call; it is recreated per unit-test loop so a session spans one FRID. --- codeplain_REST_api.py | 58 +++---- render_machine/actions/fix_unit_tests.py | 164 ++++++++++++------ render_machine/agent_tools.py | 212 +++++++++++++++++++++++ render_machine/render_types.py | 8 + tests/test_agent_tools.py | 113 ++++++++++++ tests/test_fix_unit_tests_action.py | 144 +++++++++++++++ 6 files changed, 615 insertions(+), 84 deletions(-) create mode 100644 render_machine/agent_tools.py create mode 100644 tests/test_agent_tools.py create mode 100644 tests/test_fix_unit_tests_action.py diff --git a/codeplain_REST_api.py b/codeplain_REST_api.py index a1a3a044..01d609dc 100644 --- a/codeplain_REST_api.py +++ b/codeplain_REST_api.py @@ -253,38 +253,6 @@ def render_functional_requirement( return self.post_request(endpoint_url, headers, payload, run_state) - def fix_unittests_issue( - self, - frid, - plain_source_tree, - linked_resources, - existing_files_content, - module_name: str, - required_modules, - unittests_issue, - run_state: RunState, - conformance_tests_fixes: list[dict] | None = None, - ): - endpoint_url = f"{self.api_url}/fix_unittests_issue" - headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"} - - payload = { - "frid": frid, - "plain_source_tree": plain_source_tree, - "linked_resources": linked_resources, - "existing_files_content": existing_files_content, - "module_name": module_name, - "required_modules": required_modules, - "unittests_issue": unittests_issue, - "unittest_batch_id": run_state.unittest_batch_id, - } - # Implementation code changes made by the conformance tests fixer right before this unit tests run; - # only sent when unit tests are processed inside the conformance tests phase. - if conformance_tests_fixes is not None: - payload["conformance_tests_fixes"] = conformance_tests_fixes - - return self.post_request(endpoint_url, headers, payload, run_state) - def distill_conformance_test_memory( self, frid, @@ -558,3 +526,29 @@ def summarize_finished_conformance_tests( } return self.post_request(endpoint_url, headers, payload, run_state) + + def agent_start(self, task_type: str, task_params: dict, frid: str, module_name: str, run_state: RunState): + """Start a server-side agent session; returns the first turn (tool calls or completion).""" + endpoint_url = f"{self.api_url}/agent/start" + headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"} + payload = { + "task_type": task_type, + "task_params": task_params, + "frid": frid, + "module_name": module_name, + } + return self.post_request(endpoint_url, headers, payload, run_state) + + def agent_continue( + self, session_id: str, tool_results: list[dict], frid: str, module_name: str, run_state: RunState + ): + """Feed tool results into an agent session and run its next turn.""" + endpoint_url = f"{self.api_url}/agent/continue" + headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"} + payload = { + "session_id": session_id, + "tool_results": tool_results, + "frid": frid, + "module_name": module_name, + } + return self.post_request(endpoint_url, headers, payload, run_state) diff --git a/render_machine/actions/fix_unit_tests.py b/render_machine/actions/fix_unit_tests.py index 50a54b6a..af5f1dcd 100644 --- a/render_machine/actions/fix_unit_tests.py +++ b/render_machine/actions/fix_unit_tests.py @@ -1,79 +1,139 @@ from typing import Any import file_utils -import render_machine.render_utils as render_utils +import plain_spec from plain2code_console import console from plain2code_exceptions import InternalClientError +from render_machine import agent_tools from render_machine.actions.base_action import BaseAction -from render_machine.implementation_code_helpers import ImplementationCodeHelpers from render_machine.render_context import RenderContext -MAX_ISSUE_LENGTH = 10000 +TASK_TYPE = "fix_unit_tests" +SUBMIT_FIX_TOOL = "submit_fix" +# Upper bound on LLM turns spent on one fix attempt; the server bounds the whole session. +MAX_AGENT_TURNS_PER_ATTEMPT = 40 class FixUnitTests(BaseAction): + """Fix failing unit tests with a server-side agent whose session spans all attempts for the FRID. + + The first failure starts a session; the agent then drives read/grep/edit/run tool calls + (executed here) until it calls submit_fix. The state machine re-runs the unit tests and, + if they still fail, the next execution of this action answers that submit_fix call with + the new failure output inside the same session, so earlier attempts stay in context. + """ + SUCCESSFUL_OUTCOME = "unit_tests_fix_generated" def execute(self, render_context: RenderContext, previous_action_payload: Any | None): - if not previous_action_payload.get("previous_unittests_issue"): + if not previous_action_payload or not previous_action_payload.get("previous_unittests_issue"): raise InternalClientError( "Internal client error: Previous action payload does not contain previous unit tests issue." ) - previous_unittests_issue = previous_action_payload["previous_unittests_issue"] - - if previous_unittests_issue and len(previous_unittests_issue) > MAX_ISSUE_LENGTH: - console.debug( - f"Unit tests issue text is too long and will be smartly truncated to {MAX_ISSUE_LENGTH} characters." + unittests_issue = previous_action_payload["previous_unittests_issue"] + context = render_context.unit_tests_running_context + api = render_context.codeplain_api + frid, module_name = render_context.frid_context.frid, render_context.module_name + changed_files_before = set(context.changed_files) + + if context.agent_session_id is None: + console.info("Starting an agent session to fix the unit tests.") + response = api.agent_start( + TASK_TYPE, + self._build_task_params(render_context, unittests_issue), + frid, + module_name, + render_context.run_state, ) - - existing_files, existing_files_content = ImplementationCodeHelpers.fetch_existing_files( - render_context.build_folder - ) - - render_utils.print_inputs(render_context, existing_files_content, "Files sent as input to unit tests fixing:") - - conformance_tests_fixes = self._get_conformance_tests_fixes(render_context) - if conformance_tests_fixes: - console.info( - f"Unit tests are fixed while preserving {len(conformance_tests_fixes)} implementation code change(s) " - "made to fix the conformance tests." + context.agent_session_id = response["session_id"] + else: + console.info(f"Continuing agent session {context.agent_session_id} with the new unit tests failure.") + tool_results = context.pending_tool_results + [ + { + "call_id": context.pending_submit_call_id, + "output": f"The fix was applied, but the unit tests still fail:\n{unittests_issue}", + } + ] + context.pending_tool_results, context.pending_submit_call_id = [], None + response = api.agent_continue( + context.agent_session_id, tool_results, frid, module_name, render_context.run_state ) - response_files = render_context.codeplain_api.fix_unittests_issue( - render_context.frid_context.frid, - render_context.plain_source_tree, - render_context.frid_context.linked_resources, - existing_files_content, - render_context.module_name, - render_context.get_required_modules_functionalities(), - previous_unittests_issue, - run_state=render_context.run_state, - conformance_tests_fixes=conformance_tests_fixes, - ) + submitted = False + turns = 0 + while response.get("status") == "tool_calls" and turns < MAX_AGENT_TURNS_PER_ATTEMPT: + turns += 1 + calls = response["calls"] + submit_call = next((call for call in calls if call["name"] == SUBMIT_FIX_TOOL), None) + tool_results = agent_tools.execute_calls( + [call for call in calls if call is not submit_call], render_context + ) + if submit_call is not None: + context.pending_submit_call_id = submit_call["id"] + context.pending_tool_results = tool_results + submitted = True + console.info(f"Agent submitted a fix: {submit_call['args'].get('changes_made', '')}") + break + response = api.agent_continue( + context.agent_session_id, tool_results, frid, module_name, render_context.run_state + ) - _, changed_files = file_utils.update_build_folder_with_rendered_files( - render_context.build_folder, existing_files, response_files + if not submitted: + # The session ended without a submission (finished in text, failed, or used up this + # attempt's turn budget) — a fresh session is started if the tests still fail. + status = response.get("status") + if status == "failed": + console.warning(f"Agent session failed: {response.get('error', 'unknown error')}") + elif status == "tool_calls": + console.warning(f"Agent used {MAX_AGENT_TURNS_PER_ATTEMPT} turns without submitting a fix.") + context.agent_session_id, context.pending_submit_call_id, context.pending_tool_results = None, None, [] + + console.print_files( + "Files changed while fixing unit tests:", + render_context.build_folder, + {path: "" for path in sorted(context.changed_files - changed_files_before)}, + style=console.OUTPUT_STYLE, ) - - render_context.unit_tests_running_context.changed_files.update(changed_files) - - console.print_files("Files fixed:", render_context.build_folder, response_files, style=console.OUTPUT_STYLE) - return self.SUCCESSFUL_OUTCOME, None @staticmethod - def _get_conformance_tests_fixes(render_context: RenderContext) -> list[dict] | None: - """Implementation code changes the conformance tests fixer made before these unit tests were run. - - Only present when unit tests are processed inside the conformance tests phase - the implementation - and refactoring unit test passes have no conformance tests running context. - """ - conformance_tests_running_context = getattr(render_context, "conformance_tests_running_context", None) - if conformance_tests_running_context is None: - return None + def _build_task_params(render_context: RenderContext, unittests_issue: str) -> dict: + frid = render_context.frid_context.frid + specifications, _ = plain_spec.get_specifications_for_frid(render_context.plain_source_tree, frid) + return { + "definitions": "\n".join(specifications.get(plain_spec.DEFINITIONS, [])), + "non_functional_requirements": "\n".join(specifications.get(plain_spec.NON_FUNCTIONAL_REQUIREMENTS, [])), + "functional_requirements": FixUnitTests._functional_requirements_section(render_context, specifications), + "linked_resources": render_context.frid_context.linked_resources, + "build_folder": render_context.build_folder, + "module_name": render_context.module_name, + "unittests_script_content": FixUnitTests._read_script(render_context.unittests_script), + "unittests_issue": unittests_issue, + } - implementation_code_fixes = getattr(conformance_tests_running_context, "implementation_code_fixes", None) - if not implementation_code_fixes: - return None + @staticmethod + def _functional_requirements_section(render_context: RenderContext, specifications: dict) -> str: + sections = [] + for module_name, functionalities in render_context.get_required_modules_functionalities().items(): + sections.append( + f"### Module: {module_name} (Already Implemented, for context)\n" + "\n".join(functionalities) + ) + current = specifications.get(plain_spec.FUNCTIONAL_REQUIREMENTS, []) + if len(current) > 1: + sections.append( + f"### Module: {render_context.module_name} (Already Implemented, for context)\n" + + "\n".join(current[:-1]) + ) + if current: + sections.append(f"### Module: {render_context.module_name} (Currently Being Implemented)\n{current[-1]}") + return "\n\n".join(sections) - return list(implementation_code_fixes) + @staticmethod + def _read_script(script: str | None) -> str: + if not script: + return "" + try: + with open(file_utils.add_current_path_if_no_path(script), "r", encoding="utf-8") as f: + return f.read() + except OSError: + return "" diff --git a/render_machine/agent_tools.py b/render_machine/agent_tools.py new file mode 100644 index 00000000..845b4509 --- /dev/null +++ b/render_machine/agent_tools.py @@ -0,0 +1,212 @@ +"""Client-side implementations of the tools a server-side agent can call. + +The server declares the tools to the LLM (codeplain-api: src/agent/tools.py) and forwards +the model's calls; this module executes them against the local build folder and returns +plain-text results. Relative paths resolve against the build folder. Reads are allowed in the +build folder and the project root (the CWD); writes only inside the build folder. +""" + +import glob +import os +import subprocess +import tempfile +from typing import Callable + +from plain2code_console import console +from render_machine import render_utils +from render_machine.render_context import RenderContext + +DEFAULT_READ_LIMIT = 200 +MAX_LINE_CHARS = 10_000 +MAX_OUTPUT_CHARS = 30_000 +GREP_EXCLUDED_DIRS = (".git", "__pycache__", "node_modules", ".venv", "target", "dist", "build") + + +def _build_folder(render_context: RenderContext) -> str: + return os.path.normpath(os.path.abspath(render_context.build_folder)) + + +def _resolve(file_path: str, render_context: RenderContext) -> str: + if os.path.isabs(file_path): + return os.path.normpath(file_path) + return os.path.normpath(os.path.join(_build_folder(render_context), file_path)) + + +def _within(path: str, folder: str) -> bool: + return path == folder or path.startswith(folder + os.sep) + + +def _readable(path: str, render_context: RenderContext) -> bool: + return _within(path, _build_folder(render_context)) or _within(path, os.path.normpath(os.getcwd())) + + +def _writable(path: str, render_context: RenderContext) -> bool: + return _within(path, _build_folder(render_context)) + + +def _bound(text: str) -> str: + """Cap very long lines and the total size so one tool result cannot flood the context.""" + lines = [ + line if len(line) <= MAX_LINE_CHARS else line[:MAX_LINE_CHARS] + "... [line truncated]" + for line in text.split("\n") + ] + text = "\n".join(lines) + if len(text) > MAX_OUTPUT_CHARS: + head, tail = int(MAX_OUTPUT_CHARS * 0.6), int(MAX_OUTPUT_CHARS * 0.4) + text = text[:head] + f"\n\n... [truncated {len(text) - head - tail:,} chars] ...\n\n" + text[-tail:] + return text + + +def _track_change(full_path: str, render_context: RenderContext) -> None: + relative_path = os.path.relpath(full_path, _build_folder(render_context)) + render_context.unit_tests_running_context.changed_files.add(relative_path) + + +def read_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + if not _readable(full_path, render_context): + return f"Error: read access denied for '{full_path}' (readable: build folder and project root)." + if not os.path.isfile(full_path): + return f"Error: file not found: '{full_path}'." + with open(full_path, "r", encoding="utf-8", errors="replace") as f: + lines = f.read().split("\n") + offset = max(int(args.get("offset") or 1), 1) + limit = int(args.get("limit") or DEFAULT_READ_LIMIT) + selected = lines[offset - 1 : offset - 1 + limit] + if not selected: + return f"Error: offset {offset} is past the end of the file ({len(lines)} lines)." + numbered = "\n".join(f"{offset + i}: {line}" for i, line in enumerate(selected)) + last = offset - 1 + len(selected) + note = ( + f"\n[showing lines {offset}-{last} of {len(lines)}; use offset={last + 1} to continue]" + if last < len(lines) + else "" + ) + return _bound(numbered) + note + + +def grep(args: dict, render_context: RenderContext) -> str: + pattern = args.get("pattern", "") + if not pattern: + return "Error: pattern is required." + target = _resolve(args.get("file_path") or ".", render_context) + if not _readable(target, render_context): + return f"Error: read access denied for '{target}'." + if not os.path.exists(target): + return f"Error: path not found: '{target}'." + # Run from the build folder so matches inside it come back as build-relative paths, which + # is the form the other tools accept. + build_folder = _build_folder(render_context) + cwd = build_folder if _within(target, build_folder) else os.getcwd() + command = ["grep", "-rnI", *[f"--exclude-dir={d}" for d in GREP_EXCLUDED_DIRS], "-e", pattern, "--"] + command.append(os.path.relpath(target, cwd) if _within(target, cwd) else target) + result = subprocess.run(command, capture_output=True, text=True, cwd=cwd) + if result.returncode == 1: + return f"No matches for '{pattern}' in '{target}'." + if result.returncode != 0: + return f"Error: grep failed: {result.stderr.strip()}" + lines = [line[2:] if line.startswith("./") else line for line in result.stdout.rstrip("\n").split("\n")] + return _bound("\n".join(lines)) + + +def ls_files(args: dict, render_context: RenderContext) -> str: + target = _resolve(args.get("pattern") or ".", render_context) + if not _readable(target, render_context): + return f"Error: read access denied for '{target}'." + if os.path.isdir(target): + entries = sorted(os.listdir(target)) + listing = [entry + "/" if os.path.isdir(os.path.join(target, entry)) else entry for entry in entries] + return f"{target}:\n" + ("\n".join(listing) if listing else "(empty)") + matches = sorted(glob.glob(target, recursive=True)) + return "\n".join(matches) if matches else f"No files match '{target}'." + + +def edit_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + search, replace = args.get("search", ""), args.get("replace", "") + if not search: + return "Error: search is required." + if not _writable(full_path, render_context): + return f"Error: write access denied for '{full_path}' (writable: build folder only)." + if not os.path.isfile(full_path): + return f"Error: file not found: '{full_path}'. Use write_file to create new files." + with open(full_path, "r", encoding="utf-8") as f: + content = f.read() + occurrences = content.count(search) + if occurrences != 1: + return ( + f"Error: search text found {occurrences} times in '{full_path}'; it must appear exactly once. " + "Read the file and use a larger, unique snippet." + ) + with open(full_path, "w", encoding="utf-8") as f: + f.write(content.replace(search, replace, 1)) + _track_change(full_path, render_context) + return f"Edited '{full_path}'." + + +def write_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + if not _writable(full_path, render_context): + return f"Error: write access denied for '{full_path}' (writable: build folder only)." + os.makedirs(os.path.dirname(full_path), exist_ok=True) + with open(full_path, "w", encoding="utf-8") as f: + f.write(args.get("content", "")) + _track_change(full_path, render_context) + return f"Wrote '{full_path}'." + + +def delete_file(args: dict, render_context: RenderContext) -> str: + full_path = _resolve(args.get("file_path", ""), render_context) + if not _writable(full_path, render_context): + return f"Error: write access denied for '{full_path}' (writable: build folder only)." + if not os.path.isfile(full_path): + return f"Error: file not found: '{full_path}'." + os.remove(full_path) + _track_change(full_path, render_context) + return f"Deleted '{full_path}'." + + +def run_unit_tests(_args: dict, render_context: RenderContext) -> str: + exit_code, output, log_file_path = render_utils.execute_script( + os.path.normpath(render_context.unittests_script), + [render_context.build_folder], + "Unit Tests", + timeout=render_context.test_script_timeout, + stop_event=render_context.stop_event, + ) + if exit_code == 0: + return "All unit tests passed." + if not log_file_path and output: + with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False, suffix=".unittest_output") as f: + f.write(output) + log_file_path = f.name + pointer = f" Full output: {log_file_path} (use read_file)." if log_file_path else "" + return f"Unit tests failed (exit code {exit_code}).{pointer}\n{_bound(output)}" + + +TOOLS: dict[str, Callable[[dict, RenderContext], str]] = { + "read_file": read_file, + "grep": grep, + "ls_files": ls_files, + "edit_file": edit_file, + "write_file": write_file, + "delete_file": delete_file, + "run_unit_tests": run_unit_tests, +} + + +def execute_calls(calls: list[dict], render_context: RenderContext) -> list[dict]: + """Execute the agent's tool calls in order; every call gets a result, errors included.""" + results = [] + for call in calls: + tool = TOOLS.get(call["name"]) + if tool is None: + output = f"Error: unknown tool '{call['name']}'." + else: + try: + output = tool(call.get("args") or {}, render_context) + except Exception as e: + output = f"Error: tool '{call['name']}' failed: {type(e).__name__}: {e}" + console.debug(f"Agent tool {call['name']}({call.get('args')}) -> {output[:200]!r}") + results.append({"call_id": call["id"], "output": output}) + return results diff --git a/render_machine/render_types.py b/render_machine/render_types.py index 75a2f9b6..6686d3a5 100644 --- a/render_machine/render_types.py +++ b/render_machine/render_types.py @@ -52,6 +52,14 @@ class FridContext: class UnitTestsRunningContext: fix_attempts: int changed_files: set[str] = field(default_factory=set) + # Server-side agent session fixing this FRID's unit tests. It spans every fix attempt of + # one unit-test loop (the context is recreated when the loop starts), so each new failure + # is fed back into the same conversation instead of a fresh, stateless call. + agent_session_id: Optional[str] = None + # The submit_fix call the agent ended its last attempt with, answered with the next test + # run's outcome, plus results of any tool calls made in the same turn as submit_fix. + pending_submit_call_id: Optional[str] = None + pending_tool_results: list[dict] = field(default_factory=list) class ConformanceTestsRunningContext: diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py new file mode 100644 index 00000000..f834d910 --- /dev/null +++ b/tests/test_agent_tools.py @@ -0,0 +1,113 @@ +"""Tests for the client-side agent tool implementations.""" + +import os +from types import SimpleNamespace + +import pytest + +from render_machine import agent_tools + + +@pytest.fixture +def project(tmp_path, monkeypatch): + build = tmp_path / "plain_modules" / "m" + build.mkdir(parents=True) + (build / "app.py").write_text("def add(a, b):\n return a - b\n\n\ndef sub(a, b):\n return a - b\n") + (tmp_path / "outside.txt").write_text("outside\n") + monkeypatch.chdir(tmp_path) + render_context = SimpleNamespace( + build_folder=str(build), + unit_tests_running_context=SimpleNamespace(changed_files=set()), + unittests_script=None, + test_script_timeout=None, + stop_event=None, + ) + return SimpleNamespace(root=tmp_path, build=build, rc=render_context) + + +def test_read_file_resolves_relative_to_build_folder_with_paging(project): + out = agent_tools.read_file({"file_path": "app.py", "offset": 2, "limit": 1}, project.rc) + assert out.startswith("2: return a - b") + assert "use offset=3 to continue" in out + + +def test_read_allows_project_root_but_not_elsewhere(project): + assert "outside" in agent_tools.read_file({"file_path": str(project.root / "outside.txt")}, project.rc) + assert agent_tools.read_file({"file_path": "/etc/hosts"}, project.rc).startswith("Error: read access denied") + assert agent_tools.read_file({"file_path": "missing.py"}, project.rc).startswith("Error: file not found") + + +def test_edit_file_requires_unique_match_and_tracks_change(project): + ambiguous = agent_tools.edit_file({"file_path": "app.py", "search": "return a - b", "replace": "x"}, project.rc) + assert "found 2 times" in ambiguous + assert project.rc.unit_tests_running_context.changed_files == set() + + ok = agent_tools.edit_file( + { + "file_path": "app.py", + "search": "def add(a, b):\n return a - b", + "replace": "def add(a, b):\n return a + b", + }, + project.rc, + ) + assert ok.startswith("Edited") + assert "return a + b" in (project.build / "app.py").read_text() + assert project.rc.unit_tests_running_context.changed_files == {"app.py"} + + +def test_write_and_delete_are_confined_to_build_folder(project): + denied = agent_tools.write_file({"file_path": str(project.root / "evil.py"), "content": "x"}, project.rc) + assert denied.startswith("Error: write access denied") + assert not (project.root / "evil.py").exists() + + assert agent_tools.write_file({"file_path": "pkg/new.py", "content": "print(1)\n"}, project.rc).startswith("Wrote") + assert (project.build / "pkg" / "new.py").read_text() == "print(1)\n" + assert agent_tools.delete_file({"file_path": "pkg/new.py"}, project.rc).startswith("Deleted") + assert not (project.build / "pkg" / "new.py").exists() + assert project.rc.unit_tests_running_context.changed_files == {os.path.join("pkg", "new.py")} + + +def test_grep_and_ls(project): + hits = agent_tools.grep({"pattern": "def sub"}, project.rc) + assert hits == "app.py:5:def sub(a, b):" + assert agent_tools.grep({"pattern": "nope"}, project.rc).startswith("No matches") + assert agent_tools.grep({"pattern": ""}, project.rc).startswith("Error") + + assert agent_tools.ls_files({}, project.rc).splitlines()[1:] == ["app.py"] + assert agent_tools.ls_files({"pattern": "**/*.py"}, project.rc).endswith("app.py") + + +def test_execute_calls_answers_every_call_and_captures_errors(project, monkeypatch): + def boom(_args, _rc): + raise RuntimeError("kaput") + + monkeypatch.setitem(agent_tools.TOOLS, "boom", boom) + results = agent_tools.execute_calls( + [ + {"id": "1", "name": "read_file", "args": {"file_path": "app.py", "limit": 1}}, + {"id": "2", "name": "unknown_tool", "args": {}}, + {"id": "3", "name": "boom"}, + ], + project.rc, + ) + assert [r["call_id"] for r in results] == ["1", "2", "3"] + assert results[0]["output"].startswith("1: def add") + assert results[1]["output"] == "Error: unknown tool 'unknown_tool'." + assert results[2]["output"] == "Error: tool 'boom' failed: RuntimeError: kaput" + + +def test_run_unit_tests_reports_pass_and_failure(project, monkeypatch): + outcomes = iter([(0, "", None), (1, "FAILED test_x", "/tmp/log.txt")]) + monkeypatch.setattr(agent_tools.render_utils, "execute_script", lambda *a, **k: next(outcomes)) + project.rc.unittests_script = "run_tests.sh" + assert agent_tools.run_unit_tests({}, project.rc) == "All unit tests passed." + failure = agent_tools.run_unit_tests({}, project.rc) + assert ( + failure.startswith("Unit tests failed (exit code 1). Full output: /tmp/log.txt") and "FAILED test_x" in failure + ) + + +def test_bound_truncates_long_lines_and_large_output(): + bounded = agent_tools._bound("a" * (agent_tools.MAX_LINE_CHARS + 5) + "\n" + "b\n" * 40_000) + assert "[line truncated]" in bounded and "[truncated" in bounded + assert len(bounded) < agent_tools.MAX_OUTPUT_CHARS + 200 diff --git a/tests/test_fix_unit_tests_action.py b/tests/test_fix_unit_tests_action.py new file mode 100644 index 00000000..10212079 --- /dev/null +++ b/tests/test_fix_unit_tests_action.py @@ -0,0 +1,144 @@ +"""Tests for the agentic FixUnitTests action driving a scripted fake API.""" + +from types import SimpleNamespace + +import pytest + +import plain_spec +from render_machine.actions.fix_unit_tests import MAX_AGENT_TURNS_PER_ATTEMPT, FixUnitTests +from render_machine.render_types import UnitTestsRunningContext + + +class FakeAPI: + def __init__(self, responses): + self.responses = list(responses) + self.calls = [] + + def agent_start(self, task_type, task_params, frid, module_name, run_state): + self.calls.append(("start", task_type, task_params, frid, module_name)) + return self.responses.pop(0) + + def agent_continue(self, session_id, tool_results, frid, module_name, run_state): + self.calls.append(("continue", session_id, tool_results, frid, module_name)) + return self.responses.pop(0) + + +def _tool_calls(*calls): + return {"session_id": "s1", "status": "tool_calls", "calls": list(calls)} + + +@pytest.fixture +def render_context(tmp_path, monkeypatch): + build = tmp_path / "build" + build.mkdir() + (build / "a.py").write_text("x = 1\n") + monkeypatch.chdir(tmp_path) + plain_source_tree = {"spec": True} + specifications = { + plain_spec.DEFINITIONS: ["- :Foo: is a thing."], + plain_spec.NON_FUNCTIONAL_REQUIREMENTS: ["- Python 3.11."], + plain_spec.FUNCTIONAL_REQUIREMENTS: ["- Old feature.", "- New feature."], + } + monkeypatch.setattr(plain_spec, "get_specifications_for_frid", lambda tree, frid: (specifications, None)) + return SimpleNamespace( + codeplain_api=None, + build_folder=str(build), + module_name="m", + run_state=object(), + plain_source_tree=plain_source_tree, + unittests_script=None, + test_script_timeout=None, + stop_event=None, + frid_context=SimpleNamespace(frid="2", linked_resources={"schema.json": "{}"}), + unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), + get_required_modules_functionalities=lambda: {"base": ["- Base feature."]}, + ) + + +def test_first_attempt_starts_session_runs_tools_and_stops_at_submit_fix(render_context): + api = FakeAPI( + [ + _tool_calls({"id": "c1", "name": "read_file", "args": {"file_path": "a.py"}}), + _tool_calls( + {"id": "c2", "name": "edit_file", "args": {"file_path": "a.py", "search": "x = 1", "replace": "x = 2"}}, + {"id": "c3", "name": "submit_fix", "args": {"root_cause": "off by one", "changes_made": "x = 2"}}, + ), + ] + ) + render_context.codeplain_api = api + + outcome, payload = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED test_a"}) + + assert (outcome, payload) == (FixUnitTests.SUCCESSFUL_OUTCOME, None) + kind, task_type, task_params, frid, module_name = api.calls[0] + assert (kind, task_type, frid, module_name) == ("start", "fix_unit_tests", "2", "m") + assert task_params["unittests_issue"] == "FAILED test_a" + assert task_params["definitions"] == "- :Foo: is a thing." + assert task_params["linked_resources"] == {"schema.json": "{}"} + assert ( + "### Module: base (Already Implemented, for context)\n- Base feature." in task_params["functional_requirements"] + ) + assert "### Module: m (Already Implemented, for context)\n- Old feature." in task_params["functional_requirements"] + assert task_params["functional_requirements"].endswith( + "### Module: m (Currently Being Implemented)\n- New feature." + ) + + # the read_file result went back to the server; the edit was applied locally + assert api.calls[1][0] == "continue" and api.calls[1][2][0]["call_id"] == "c1" + assert "1: x = 1" in api.calls[1][2][0]["output"] + assert open(render_context.build_folder + "/a.py").read() == "x = 2\n" + + context = render_context.unit_tests_running_context + assert context.agent_session_id == "s1" + assert context.pending_submit_call_id == "c3" + assert [r["call_id"] for r in context.pending_tool_results] == ["c2"] + assert context.changed_files == {"a.py"} + + +def test_second_attempt_continues_session_answering_submit_fix(render_context): + context = render_context.unit_tests_running_context + context.agent_session_id = "s1" + context.pending_submit_call_id = "c3" + context.pending_tool_results = [{"call_id": "c2", "output": "Edited"}] + api = FakeAPI([_tool_calls({"id": "c4", "name": "submit_fix", "args": {"changes_made": "again"}})]) + render_context.codeplain_api = api + + outcome, _ = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED test_b"}) + + assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME + kind, session_id, tool_results, frid, module_name = api.calls[0] + assert (kind, session_id, frid, module_name) == ("continue", "s1", "2", "m") + assert tool_results[0] == {"call_id": "c2", "output": "Edited"} + assert tool_results[1]["call_id"] == "c3" and "FAILED test_b" in tool_results[1]["output"] + assert context.agent_session_id == "s1" and context.pending_submit_call_id == "c4" + assert context.pending_tool_results == [] + + +@pytest.mark.parametrize( + "final_response", + [ + {"session_id": "s1", "status": "completed", "result": "done"}, + {"session_id": "s1", "status": "failed", "error": "x"}, + ], +) +def test_session_ending_without_submission_resets_the_session(render_context, final_response): + render_context.codeplain_api = FakeAPI([final_response]) + outcome, _ = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME + context = render_context.unit_tests_running_context + assert context.agent_session_id is None and context.pending_submit_call_id is None + + +def test_turn_cap_per_attempt_resets_the_session(render_context): + call = {"id": "c", "name": "ls_files", "args": {}} + render_context.codeplain_api = FakeAPI([_tool_calls(call)] * (MAX_AGENT_TURNS_PER_ATTEMPT + 1)) + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + assert len(render_context.codeplain_api.calls) == MAX_AGENT_TURNS_PER_ATTEMPT + 1 + assert render_context.unit_tests_running_context.agent_session_id is None + + +def test_missing_issue_is_an_internal_error(render_context): + from plain2code_exceptions import InternalClientError + + with pytest.raises(InternalClientError): + FixUnitTests().execute(render_context, {}) From 2d336fd16c6371ab9943d61163d2381f61976299 Mon Sep 17 00:00:00 2001 From: VitjanZ Date: Fri, 25 Sep 2026 12:21:20 +0200 Subject: [PATCH 2/5] Speed up the agentic unit-test fixer - Send raw test output as test_output (run_unit_tests and the submit_fix follow-up) so the server condenses it, instead of head/tail truncation that can drop the root cause. - Make the full test logs readable by read_file/grep and point the agent at them; grep gains context_lines and include. - Seed the first turn with the build folder's file tree and the files changed for the current FRID. - Skip the harness unit-test run when the agent's own run passed and no file changed since. - Start a new session after an abandoned one with previous_session_id so the server can pass on what was tried. - edit_file returns the edited region; repeated read-only calls with no file change in between return a pointer instead of the same output again. --- render_machine/actions/fix_unit_tests.py | 52 +++++++++++++- render_machine/actions/run_unit_tests.py | 9 +++ render_machine/agent_tools.py | 92 +++++++++++++++++++----- render_machine/render_types.py | 12 ++++ tests/test_agent_tools.py | 53 ++++++++++++-- tests/test_fix_unit_tests_action.py | 61 ++++++++++++++-- 6 files changed, 252 insertions(+), 27 deletions(-) diff --git a/render_machine/actions/fix_unit_tests.py b/render_machine/actions/fix_unit_tests.py index af5f1dcd..f8d9f9d2 100644 --- a/render_machine/actions/fix_unit_tests.py +++ b/render_machine/actions/fix_unit_tests.py @@ -1,3 +1,4 @@ +import os from typing import Any import file_utils @@ -12,6 +13,9 @@ SUBMIT_FIX_TOOL = "submit_fix" # Upper bound on LLM turns spent on one fix attempt; the server bounds the whole session. MAX_AGENT_TURNS_PER_ATTEMPT = 40 +# Seeding the first turn: the build folder's file list and the files changed for the FRID. +MAX_FILE_TREE_ENTRIES = 500 +MAX_RELEVANT_FILES_CHARS = 60_000 class FixUnitTests(BaseAction): @@ -35,9 +39,14 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | api = render_context.codeplain_api frid, module_name = render_context.frid_context.frid, render_context.module_name changed_files_before = set(context.changed_files) + log_path = render_context.script_execution_history.latest_unit_test_output_path + if log_path: + agent_tools.register_log_path(log_path, render_context) if context.agent_session_id is None: console.info("Starting an agent session to fix the unit tests.") + # Cached read results point at earlier turns, which a new session does not have. + context.tool_result_cache.clear() response = api.agent_start( TASK_TYPE, self._build_task_params(render_context, unittests_issue), @@ -51,7 +60,9 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | tool_results = context.pending_tool_results + [ { "call_id": context.pending_submit_call_id, - "output": f"The fix was applied, but the unit tests still fail:\n{unittests_issue}", + "output": "The fix was applied, but the unit tests still fail." + + agent_tools.full_log_pointer(log_path), + "test_output": unittests_issue, } ] context.pending_tool_results, context.pending_submit_call_id = [], None @@ -86,6 +97,7 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | console.warning(f"Agent session failed: {response.get('error', 'unknown error')}") elif status == "tool_calls": console.warning(f"Agent used {MAX_AGENT_TURNS_PER_ATTEMPT} turns without submitting a fix.") + context.previous_session_id = context.agent_session_id context.agent_session_id, context.pending_submit_call_id, context.pending_tool_results = None, None, [] console.print_files( @@ -100,7 +112,8 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | def _build_task_params(render_context: RenderContext, unittests_issue: str) -> dict: frid = render_context.frid_context.frid specifications, _ = plain_spec.get_specifications_for_frid(render_context.plain_source_tree, frid) - return { + context = render_context.unit_tests_running_context + task_params = { "definitions": "\n".join(specifications.get(plain_spec.DEFINITIONS, [])), "non_functional_requirements": "\n".join(specifications.get(plain_spec.NON_FUNCTIONAL_REQUIREMENTS, [])), "functional_requirements": FixUnitTests._functional_requirements_section(render_context, specifications), @@ -109,7 +122,42 @@ def _build_task_params(render_context: RenderContext, unittests_issue: str) -> d "module_name": render_context.module_name, "unittests_script_content": FixUnitTests._read_script(render_context.unittests_script), "unittests_issue": unittests_issue, + "unittests_log_path": render_context.script_execution_history.latest_unit_test_output_path, + "file_tree": FixUnitTests._file_tree(render_context.build_folder), + "relevant_files": FixUnitTests._relevant_files( + render_context.build_folder, render_context.frid_context.changed_files | context.changed_files + ), } + if context.previous_session_id: + task_params["previous_session_id"] = context.previous_session_id + return task_params + + @staticmethod + def _file_tree(build_folder: str) -> str: + paths = [] + for root, dirs, files in os.walk(build_folder): + dirs[:] = sorted(d for d in dirs if d not in agent_tools.GREP_EXCLUDED_DIRS and not d.startswith(".")) + paths.extend(os.path.relpath(os.path.join(root, name), build_folder) for name in sorted(files)) + if len(paths) > MAX_FILE_TREE_ENTRIES: + return "\n".join(paths[:MAX_FILE_TREE_ENTRIES]) + "\n... [more files not listed]" + return "\n".join(paths) + + @staticmethod + def _relevant_files(build_folder: str, file_names: set[str]) -> dict[str, str]: + """Contents of the given build-relative files, smallest first, within MAX_RELEVANT_FILES_CHARS.""" + contents = {} + for name in file_names: + full_path = os.path.join(build_folder, name) + if os.path.isfile(full_path): + with open(full_path, "r", encoding="utf-8", errors="replace") as f: + contents[name] = f.read() + relevant, total = {}, 0 + for name in sorted(contents, key=lambda n: (len(contents[n]), n)): + if total + len(contents[name]) > MAX_RELEVANT_FILES_CHARS: + break + relevant[name] = contents[name] + total += len(contents[name]) + return dict(sorted(relevant.items())) @staticmethod def _functional_requirements_section(render_context: RenderContext, specifications: dict) -> str: diff --git a/render_machine/actions/run_unit_tests.py b/render_machine/actions/run_unit_tests.py index 79e29dbc..93b0e828 100644 --- a/render_machine/actions/run_unit_tests.py +++ b/render_machine/actions/run_unit_tests.py @@ -16,6 +16,15 @@ class RunUnitTests(BaseAction): UNRECOVERABLE_ERROR_OUTCOME = "unrecoverable_error_occurred" def execute(self, render_context: RenderContext, _previous_action_payload: Any | None): + context = render_context.unit_tests_running_context + if context.verified_passing: + # The fixing agent's own run passed and no file changed since; running again is redundant. + context.verified_passing = False + console.info("Unit tests already passed in the fixing agent's last run; not running them again.") + render_context.script_execution_history.latest_unit_test_output_path = context.verified_passing_log_path + render_context.script_execution_history.should_update_script_outputs = True + return self.SUCCESSFUL_OUTCOME, None + unittests_script = os.path.normpath(render_context.unittests_script) console.info( diff --git a/render_machine/agent_tools.py b/render_machine/agent_tools.py index 845b4509..53df2c70 100644 --- a/render_machine/agent_tools.py +++ b/render_machine/agent_tools.py @@ -7,6 +7,7 @@ """ import glob +import json import os import subprocess import tempfile @@ -19,6 +20,11 @@ DEFAULT_READ_LIMIT = 200 MAX_LINE_CHARS = 10_000 MAX_OUTPUT_CHARS = 30_000 +EDIT_SNIPPET_CONTEXT_LINES = 3 +MAX_EDIT_SNIPPET_LINES = 60 +MAX_GREP_CONTEXT_LINES = 20 +# Tools without side effects; repeating one while no file changed returns a pointer instead. +READ_ONLY_TOOLS = ("read_file", "grep", "ls_files") GREP_EXCLUDED_DIRS = (".git", "__pycache__", "node_modules", ".venv", "target", "dist", "build") @@ -37,7 +43,16 @@ def _within(path: str, folder: str) -> bool: def _readable(path: str, render_context: RenderContext) -> bool: - return _within(path, _build_folder(render_context)) or _within(path, os.path.normpath(os.getcwd())) + return ( + _within(path, _build_folder(render_context)) + or _within(path, os.path.normpath(os.getcwd())) + or path in render_context.unit_tests_running_context.readable_log_paths + ) + + +def register_log_path(log_path: str, render_context: RenderContext) -> None: + """Allow read_file/grep on a full test log the agent is pointed to.""" + render_context.unit_tests_running_context.readable_log_paths.add(os.path.normpath(os.path.abspath(log_path))) def _writable(path: str, render_context: RenderContext) -> bool: @@ -58,8 +73,10 @@ def _bound(text: str) -> str: def _track_change(full_path: str, render_context: RenderContext) -> None: - relative_path = os.path.relpath(full_path, _build_folder(render_context)) - render_context.unit_tests_running_context.changed_files.add(relative_path) + context = render_context.unit_tests_running_context + context.changed_files.add(os.path.relpath(full_path, _build_folder(render_context))) + context.verified_passing, context.verified_passing_log_path = False, None + context.tool_result_cache.clear() def read_file(args: dict, render_context: RenderContext) -> str: @@ -98,7 +115,13 @@ def grep(args: dict, render_context: RenderContext) -> str: # is the form the other tools accept. build_folder = _build_folder(render_context) cwd = build_folder if _within(target, build_folder) else os.getcwd() - command = ["grep", "-rnI", *[f"--exclude-dir={d}" for d in GREP_EXCLUDED_DIRS], "-e", pattern, "--"] + options = [f"--exclude-dir={d}" for d in GREP_EXCLUDED_DIRS] + context_lines = min(max(int(args.get("context_lines") or 0), 0), MAX_GREP_CONTEXT_LINES) + if context_lines: + options.append(f"-C{context_lines}") + if args.get("include"): + options.append(f"--include={args['include']}") + command = ["grep", "-rnI", *options, "-e", pattern, "--"] command.append(os.path.relpath(target, cwd) if _within(target, cwd) else target) result = subprocess.run(command, capture_output=True, text=True, cwd=cwd) if result.returncode == 1: @@ -118,7 +141,7 @@ def ls_files(args: dict, render_context: RenderContext) -> str: listing = [entry + "/" if os.path.isdir(os.path.join(target, entry)) else entry for entry in entries] return f"{target}:\n" + ("\n".join(listing) if listing else "(empty)") matches = sorted(glob.glob(target, recursive=True)) - return "\n".join(matches) if matches else f"No files match '{target}'." + return _bound("\n".join(matches)) if matches else f"No files match '{target}'." def edit_file(args: dict, render_context: RenderContext) -> str: @@ -141,7 +164,18 @@ def edit_file(args: dict, render_context: RenderContext) -> str: with open(full_path, "w", encoding="utf-8") as f: f.write(content.replace(search, replace, 1)) _track_change(full_path, render_context) - return f"Edited '{full_path}'." + return f"Edited '{full_path}'. The edited region now reads:\n" + _edit_snippet(content, search, replace) + + +def _edit_snippet(content: str, search: str, replace: str) -> str: + """Numbered lines of the replacement plus a little context, so the edit needs no re-read.""" + lines = content.replace(search, replace, 1).split("\n") + first = content[: content.index(search)].count("\n") + last = first + replace.count("\n") + start = max(first - EDIT_SNIPPET_CONTEXT_LINES, 0) + end = min(last + EDIT_SNIPPET_CONTEXT_LINES + 1, len(lines), start + MAX_EDIT_SNIPPET_LINES) + snippet = "\n".join(f"{start + i + 1}: {line}" for i, line in enumerate(lines[start:end])) + return _bound(snippet) def write_file(args: dict, render_context: RenderContext) -> str: @@ -166,7 +200,13 @@ def delete_file(args: dict, render_context: RenderContext) -> str: return f"Deleted '{full_path}'." -def run_unit_tests(_args: dict, render_context: RenderContext) -> str: +def full_log_pointer(log_file_path: str | None) -> str: + return f" Full log: {log_file_path} (search it with grep, passing that path as file_path)." if log_file_path else "" + + +def run_unit_tests(_args: dict, render_context: RenderContext) -> dict: + """Returns a result dict: a short header in `output` and the raw failure output in + `test_output`, which the server condenses (summarizing it when it is long).""" exit_code, output, log_file_path = render_utils.execute_script( os.path.normpath(render_context.unittests_script), [render_context.build_folder], @@ -174,17 +214,23 @@ def run_unit_tests(_args: dict, render_context: RenderContext) -> str: timeout=render_context.test_script_timeout, stop_event=render_context.stop_event, ) + context = render_context.unit_tests_running_context if exit_code == 0: - return "All unit tests passed." + context.verified_passing, context.verified_passing_log_path = True, log_file_path + return {"output": "All unit tests passed."} if not log_file_path and output: with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False, suffix=".unittest_output") as f: f.write(output) log_file_path = f.name - pointer = f" Full output: {log_file_path} (use read_file)." if log_file_path else "" - return f"Unit tests failed (exit code {exit_code}).{pointer}\n{_bound(output)}" + if log_file_path: + register_log_path(log_file_path, render_context) + return { + "output": f"Unit tests failed (exit code {exit_code}).{full_log_pointer(log_file_path)}", + "test_output": output, + } -TOOLS: dict[str, Callable[[dict, RenderContext], str]] = { +TOOLS: dict[str, Callable[[dict, RenderContext], str | dict]] = { "read_file": read_file, "grep": grep, "ls_files": ls_files, @@ -196,17 +242,31 @@ def run_unit_tests(_args: dict, render_context: RenderContext) -> str: def execute_calls(calls: list[dict], render_context: RenderContext) -> list[dict]: - """Execute the agent's tool calls in order; every call gets a result, errors included.""" + """Execute the agent's tool calls in order; every call gets a result, errors included. + + A repeated read-only call (same tool and arguments, no file changed in between) is not + re-executed: its result is already in the conversation, so a short pointer is returned.""" + cache = render_context.unit_tests_running_context.tool_result_cache results = [] for call in calls: tool = TOOLS.get(call["name"]) + cache_key = json.dumps([call["name"], call.get("args") or {}], sort_keys=True) + result: dict if tool is None: - output = f"Error: unknown tool '{call['name']}'." + result = {"output": f"Error: unknown tool '{call['name']}'."} + elif call["name"] in READ_ONLY_TOOLS and cache_key in cache: + result = { + "output": "Same call as an earlier one and no file has changed since; " + "its result is unchanged (see the earlier result above)." + } else: try: output = tool(call.get("args") or {}, render_context) + result = output if isinstance(output, dict) else {"output": output} except Exception as e: - output = f"Error: tool '{call['name']}' failed: {type(e).__name__}: {e}" - console.debug(f"Agent tool {call['name']}({call.get('args')}) -> {output[:200]!r}") - results.append({"call_id": call["id"], "output": output}) + result = {"output": f"Error: tool '{call['name']}' failed: {type(e).__name__}: {e}"} + if call["name"] in READ_ONLY_TOOLS and not result["output"].startswith("Error"): + cache[cache_key] = result["output"] + console.debug(f"Agent tool {call['name']}({call.get('args')}) -> {result['output'][:200]!r}") + results.append({"call_id": call["id"], **result}) return results diff --git a/render_machine/render_types.py b/render_machine/render_types.py index 6686d3a5..5455422e 100644 --- a/render_machine/render_types.py +++ b/render_machine/render_types.py @@ -60,6 +60,18 @@ class UnitTestsRunningContext: # run's outcome, plus results of any tool calls made in the same turn as submit_fix. pending_submit_call_id: Optional[str] = None pending_tool_results: list[dict] = field(default_factory=list) + # Session abandoned without a submission (turn budget used up, LLM failure); the next session + # starts with a digest of what it tried. + previous_session_id: Optional[str] = None + # Set when the agent's own run_unit_tests passed and no file changed since, so the harness + # can accept the fix without running the suite again. + verified_passing: bool = False + verified_passing_log_path: Optional[str] = None + # Full test logs the agent was pointed to; readable by read_file/grep although outside the + # build folder. + readable_log_paths: set[str] = field(default_factory=set) + # Results of read-only tool calls, keyed by call; cleared whenever a file changes. + tool_result_cache: dict[str, str] = field(default_factory=dict) class ConformanceTestsRunningContext: diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py index f834d910..dd19fb29 100644 --- a/tests/test_agent_tools.py +++ b/tests/test_agent_tools.py @@ -6,6 +6,7 @@ import pytest from render_machine import agent_tools +from render_machine.render_types import UnitTestsRunningContext @pytest.fixture @@ -17,7 +18,7 @@ def project(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) render_context = SimpleNamespace( build_folder=str(build), - unit_tests_running_context=SimpleNamespace(changed_files=set()), + unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), unittests_script=None, test_script_timeout=None, stop_event=None, @@ -97,14 +98,56 @@ def boom(_args, _rc): def test_run_unit_tests_reports_pass_and_failure(project, monkeypatch): - outcomes = iter([(0, "", None), (1, "FAILED test_x", "/tmp/log.txt")]) + log = project.root.parent / "log.txt" + log.write_text("FAILED test_x\nCaused by: boom\n") + outcomes = iter([(0, "", "/tmp/pass.txt"), (1, "FAILED test_x", str(log))]) monkeypatch.setattr(agent_tools.render_utils, "execute_script", lambda *a, **k: next(outcomes)) project.rc.unittests_script = "run_tests.sh" - assert agent_tools.run_unit_tests({}, project.rc) == "All unit tests passed." + context = project.rc.unit_tests_running_context + + assert agent_tools.run_unit_tests({}, project.rc) == {"output": "All unit tests passed."} + assert context.verified_passing and context.verified_passing_log_path == "/tmp/pass.txt" + failure = agent_tools.run_unit_tests({}, project.rc) - assert ( - failure.startswith("Unit tests failed (exit code 1). Full output: /tmp/log.txt") and "FAILED test_x" in failure + assert failure["output"].startswith(f"Unit tests failed (exit code 1). Full log: {log}") + # raw output goes to the server for condensing, not truncated here + assert failure["test_output"] == "FAILED test_x" + # the full log is outside the build folder and project root but greppable + assert agent_tools.grep({"pattern": "Caused by", "file_path": str(log)}, project.rc).endswith("Caused by: boom") + + +def test_file_change_invalidates_verified_pass_and_read_cache(project): + context = project.rc.unit_tests_running_context + context.verified_passing = True + first = agent_tools.execute_calls([{"id": "1", "name": "read_file", "args": {"file_path": "app.py"}}], project.rc) + repeat = agent_tools.execute_calls([{"id": "2", "name": "read_file", "args": {"file_path": "app.py"}}], project.rc) + assert first[0]["output"].startswith("1: def add") + assert repeat[0]["output"].startswith("Same call as an earlier one") + + agent_tools.write_file({"file_path": "other.py", "content": "y = 1\n"}, project.rc) + assert context.verified_passing is False + again = agent_tools.execute_calls([{"id": "3", "name": "read_file", "args": {"file_path": "app.py"}}], project.rc) + assert again[0]["output"].startswith("1: def add") + + +def test_edit_file_returns_the_edited_region(project): + out = agent_tools.edit_file( + { + "file_path": "app.py", + "search": "def sub(a, b):\n return a - b", + "replace": "def sub(a, b):\n return b", + }, + project.rc, ) + assert out.startswith("Edited") and "5: def sub(a, b):\n6: return b" in out + assert "2: return a - b" in out and "1: def add" not in out + + +def test_grep_context_lines_and_include(project): + (project.build / "notes.txt").write_text("def sub is documented here\n") + hits = agent_tools.grep({"pattern": "def sub", "context_lines": 1, "include": "*.py"}, project.rc) + assert "notes.txt" not in hits + assert "app.py-4-" in hits and "app.py:5:def sub(a, b):" in hits and "app.py-6-" in hits def test_bound_truncates_long_lines_and_large_output(): diff --git a/tests/test_fix_unit_tests_action.py b/tests/test_fix_unit_tests_action.py index 10212079..4deda172 100644 --- a/tests/test_fix_unit_tests_action.py +++ b/tests/test_fix_unit_tests_action.py @@ -5,8 +5,9 @@ import pytest import plain_spec -from render_machine.actions.fix_unit_tests import MAX_AGENT_TURNS_PER_ATTEMPT, FixUnitTests -from render_machine.render_types import UnitTestsRunningContext +from render_machine.actions.fix_unit_tests import MAX_AGENT_TURNS_PER_ATTEMPT, MAX_RELEVANT_FILES_CHARS, FixUnitTests +from render_machine.actions.run_unit_tests import RunUnitTests +from render_machine.render_types import ScriptExecutionHistory, UnitTestsRunningContext class FakeAPI: @@ -49,8 +50,9 @@ def render_context(tmp_path, monkeypatch): unittests_script=None, test_script_timeout=None, stop_event=None, - frid_context=SimpleNamespace(frid="2", linked_resources={"schema.json": "{}"}), + frid_context=SimpleNamespace(frid="2", linked_resources={"schema.json": "{}"}, changed_files={"a.py"}), unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), + script_execution_history=ScriptExecutionHistory(), get_required_modules_functionalities=lambda: {"base": ["- Base feature."]}, ) @@ -109,7 +111,8 @@ def test_second_attempt_continues_session_answering_submit_fix(render_context): kind, session_id, tool_results, frid, module_name = api.calls[0] assert (kind, session_id, frid, module_name) == ("continue", "s1", "2", "m") assert tool_results[0] == {"call_id": "c2", "output": "Edited"} - assert tool_results[1]["call_id"] == "c3" and "FAILED test_b" in tool_results[1]["output"] + assert tool_results[1]["call_id"] == "c3" and "still fail" in tool_results[1]["output"] + assert tool_results[1]["test_output"] == "FAILED test_b" assert context.agent_session_id == "s1" and context.pending_submit_call_id == "c4" assert context.pending_tool_results == [] @@ -142,3 +145,53 @@ def test_missing_issue_is_an_internal_error(render_context): with pytest.raises(InternalClientError): FixUnitTests().execute(render_context, {}) + + +def test_first_turn_is_seeded_with_file_tree_relevant_files_and_log_path(render_context, tmp_path): + (tmp_path / "build" / "tests").mkdir() + (tmp_path / "build" / "tests" / "test_a.py").write_text("assert True\n") + log = tmp_path / "unit.log" + log.write_text("full log\nCaused by: boom\n") + render_context.script_execution_history.latest_unit_test_output_path = str(log) + api = FakeAPI([{"session_id": "s1", "status": "completed", "result": "done"}]) + render_context.codeplain_api = api + + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + + task_params = api.calls[0][2] + assert task_params["file_tree"].split("\n") == ["a.py", "tests/test_a.py"] + assert task_params["relevant_files"] == {"a.py": "x = 1\n"} + assert task_params["unittests_log_path"] == str(log) + assert "previous_session_id" not in task_params + # the agent may grep the full log although it is outside the build folder and project root + assert str(log) in render_context.unit_tests_running_context.readable_log_paths + + +def test_relevant_files_stay_within_budget(tmp_path): + (tmp_path / "small.py").write_text("s") + (tmp_path / "big.py").write_text("b" * MAX_RELEVANT_FILES_CHARS) + assert FixUnitTests._relevant_files(str(tmp_path), {"small.py", "big.py", "deleted.py"}) == {"small.py": "s"} + + +def test_new_session_after_abandoned_one_references_it(render_context): + call = {"id": "c", "name": "ls_files", "args": {}} + render_context.codeplain_api = FakeAPI([_tool_calls(call)] * (MAX_AGENT_TURNS_PER_ATTEMPT + 1)) + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + assert render_context.unit_tests_running_context.previous_session_id == "s1" + + api = FakeAPI([{"session_id": "s2", "status": "completed", "result": "done"}]) + render_context.codeplain_api = api + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED again"}) + assert api.calls[0][0] == "start" and api.calls[0][2]["previous_session_id"] == "s1" + + +def test_run_unit_tests_action_skips_the_suite_after_a_verified_agent_run(render_context, monkeypatch): + import render_machine.render_utils as render_utils + + context = render_context.unit_tests_running_context + context.verified_passing, context.verified_passing_log_path = True, "/logs/pass.log" + monkeypatch.setattr(render_utils, "execute_script", lambda *a, **k: pytest.fail("suite must not run")) + + assert RunUnitTests().execute(render_context, None) == (RunUnitTests.SUCCESSFUL_OUTCOME, None) + assert context.verified_passing is False + assert render_context.script_execution_history.latest_unit_test_output_path == "/logs/pass.log" From 0639b9f49c26718798a2fea6f757547b76369c06 Mon Sep 17 00:00:00 2001 From: VitjanZ Date: Sat, 26 Sep 2026 10:01:31 +0200 Subject: [PATCH 3/5] Keep one unit-test agent session per conformance phase and hand it the conformance fixes After the conformance tests fixer changed implementation code, the unit tests failed and a fresh agent session "repaired" the implementation back to what the stale unit tests asserted. The conformance fixer re-applied its change and the loop repeated (17 times on one FRID in render b6f3ff73), each new session unaware it had already done this. - The agent session state moves into UnitTestsAgentSession. Outside the conformance phase it lives on the unit-tests running context (one session per loop, as before); during the conformance phase it lives on the conformance tests running context, so one session spans every unit-test loop of the phase (RenderContext.unit_tests_agent_session). - A new session gets the conformance tests fixes recorded so far (implementation_code_fixes) as task_params.conformance_tests_fixes, with the files they changed seeded into relevant_files. - When a later unit-test loop continues the session, the pending submit_fix is answered with "your fix was accepted, then the code was changed to fix the conformance tests", only the fixes the session has not seen yet, and the new failure output. - The conformance-context tests are rewritten for the agentic path (they still targeted the stateless /fix_unittests_issue call and errored). --- render_machine/actions/fix_unit_tests.py | 102 ++++++++---- render_machine/agent_tools.py | 4 +- render_machine/render_context.py | 9 ++ render_machine/render_types.py | 45 ++++-- tests/test_agent_tools.py | 8 +- tests/test_fix_unit_tests_action.py | 41 +++-- ...test_fix_unit_tests_conformance_context.py | 145 +++++++++++++++--- 7 files changed, 274 insertions(+), 80 deletions(-) diff --git a/render_machine/actions/fix_unit_tests.py b/render_machine/actions/fix_unit_tests.py index f8d9f9d2..f6485059 100644 --- a/render_machine/actions/fix_unit_tests.py +++ b/render_machine/actions/fix_unit_tests.py @@ -13,18 +13,28 @@ SUBMIT_FIX_TOOL = "submit_fix" # Upper bound on LLM turns spent on one fix attempt; the server bounds the whole session. MAX_AGENT_TURNS_PER_ATTEMPT = 40 +# Answer to a submit_fix whose fix was accepted in an earlier unit-test loop of the conformance phase. +ACCEPTED_THEN_CHANGED_MESSAGE = ( + "Your fix was accepted: the unit tests passed. Afterwards the implementation code was changed to fix the " + "conformance tests{see_below}, and the unit tests now fail again. Files may have changed since you last read " + "them." +) # Seeding the first turn: the build folder's file list and the files changed for the FRID. MAX_FILE_TREE_ENTRIES = 500 MAX_RELEVANT_FILES_CHARS = 60_000 class FixUnitTests(BaseAction): - """Fix failing unit tests with a server-side agent whose session spans all attempts for the FRID. + """Fix failing unit tests with a server-side agent session that spans the fix attempts. The first failure starts a session; the agent then drives read/grep/edit/run tool calls (executed here) until it calls submit_fix. The state machine re-runs the unit tests and, if they still fail, the next execution of this action answers that submit_fix call with the new failure output inside the same session, so earlier attempts stay in context. + + During the conformance phase the session also spans unit-test loops: after the conformance + tests fixer changes implementation code, the unit tests fail again and the same session is + continued, told what the conformance tests fixer changed (which it must preserve). """ SUCCESSFUL_OUTCOME = "unit_tests_fix_generated" @@ -36,39 +46,61 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | ) unittests_issue = previous_action_payload["previous_unittests_issue"] context = render_context.unit_tests_running_context + session = render_context.unit_tests_agent_session api = render_context.codeplain_api frid, module_name = render_context.frid_context.frid, render_context.module_name changed_files_before = set(context.changed_files) log_path = render_context.script_execution_history.latest_unit_test_output_path if log_path: agent_tools.register_log_path(log_path, render_context) + conformance_tests_fixes = self._get_conformance_tests_fixes(render_context) + new_conformance_tests_fixes = conformance_tests_fixes[session.conformance_fixes_handed_off :] + if new_conformance_tests_fixes: + console.info( + f"Unit tests are fixed while preserving {len(new_conformance_tests_fixes)} implementation code " + "change(s) made to fix the conformance tests." + ) - if context.agent_session_id is None: + if session.session_id is None: console.info("Starting an agent session to fix the unit tests.") # Cached read results point at earlier turns, which a new session does not have. context.tool_result_cache.clear() response = api.agent_start( TASK_TYPE, - self._build_task_params(render_context, unittests_issue), + self._build_task_params(render_context, unittests_issue, conformance_tests_fixes), frid, module_name, render_context.run_state, ) - context.agent_session_id = response["session_id"] + session.session_id = response["session_id"] else: - console.info(f"Continuing agent session {context.agent_session_id} with the new unit tests failure.") - tool_results = context.pending_tool_results + [ - { - "call_id": context.pending_submit_call_id, - "output": "The fix was applied, but the unit tests still fail." - + agent_tools.full_log_pointer(log_path), - "test_output": unittests_issue, - } - ] - context.pending_tool_results, context.pending_submit_call_id = [], None - response = api.agent_continue( - context.agent_session_id, tool_results, frid, module_name, render_context.run_state - ) + if context.agent_used_in_this_loop: + console.info(f"Continuing agent session {session.session_id} with the new unit tests failure.") + output = "The fix was applied, but the unit tests still fail." + else: + # The session's last fix was accepted in an earlier unit-test loop of this conformance + # phase; since then the conformance tests fixer changed the code and the tests fail again. + console.info( + f"Continuing agent session {session.session_id}: the unit tests fail again after the " + "implementation was changed to fix the conformance tests." + ) + output = ACCEPTED_THEN_CHANGED_MESSAGE.format( + see_below=" (see the Conformance Tests Fix below)" if new_conformance_tests_fixes else "" + ) + # Files changed outside the session, so earlier read results are stale. + context.tool_result_cache.clear() + submit_result: dict = { + "call_id": session.pending_submit_call_id, + "output": output + agent_tools.full_log_pointer(log_path), + "test_output": unittests_issue, + } + if new_conformance_tests_fixes: + submit_result["conformance_tests_fixes"] = new_conformance_tests_fixes + tool_results = session.pending_tool_results + [submit_result] + session.pending_tool_results, session.pending_submit_call_id = [], None + response = api.agent_continue(session.session_id, tool_results, frid, module_name, render_context.run_state) + session.conformance_fixes_handed_off = len(conformance_tests_fixes) + context.agent_used_in_this_loop = True submitted = False turns = 0 @@ -80,14 +112,12 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | [call for call in calls if call is not submit_call], render_context ) if submit_call is not None: - context.pending_submit_call_id = submit_call["id"] - context.pending_tool_results = tool_results + session.pending_submit_call_id = submit_call["id"] + session.pending_tool_results = tool_results submitted = True console.info(f"Agent submitted a fix: {submit_call['args'].get('changes_made', '')}") break - response = api.agent_continue( - context.agent_session_id, tool_results, frid, module_name, render_context.run_state - ) + response = api.agent_continue(session.session_id, tool_results, frid, module_name, render_context.run_state) if not submitted: # The session ended without a submission (finished in text, failed, or used up this @@ -97,8 +127,7 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | console.warning(f"Agent session failed: {response.get('error', 'unknown error')}") elif status == "tool_calls": console.warning(f"Agent used {MAX_AGENT_TURNS_PER_ATTEMPT} turns without submitting a fix.") - context.previous_session_id = context.agent_session_id - context.agent_session_id, context.pending_submit_call_id, context.pending_tool_results = None, None, [] + session.reset() console.print_files( "Files changed while fixing unit tests:", @@ -109,10 +138,23 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | return self.SUCCESSFUL_OUTCOME, None @staticmethod - def _build_task_params(render_context: RenderContext, unittests_issue: str) -> dict: + def _get_conformance_tests_fixes(render_context: RenderContext) -> list[dict]: + """Implementation code changes the conformance tests fixer made during this conformance phase, oldest + first. Empty outside the conformance phase (the implementation and refactoring unit-test loops).""" + conformance_tests_running_context = getattr(render_context, "conformance_tests_running_context", None) + if conformance_tests_running_context is None: + return [] + return list(getattr(conformance_tests_running_context, "implementation_code_fixes", None) or []) + + @staticmethod + def _build_task_params( + render_context: RenderContext, unittests_issue: str, conformance_tests_fixes: list[dict] + ) -> dict: frid = render_context.frid_context.frid specifications, _ = plain_spec.get_specifications_for_frid(render_context.plain_source_tree, frid) context = render_context.unit_tests_running_context + # The files the conformance tests fixes changed are always seeded, so the agent sees what to preserve. + conformance_fix_files = {name for fix in conformance_tests_fixes for name in (fix or {}).get("code_diff") or {}} task_params = { "definitions": "\n".join(specifications.get(plain_spec.DEFINITIONS, [])), "non_functional_requirements": "\n".join(specifications.get(plain_spec.NON_FUNCTIONAL_REQUIREMENTS, [])), @@ -125,11 +167,15 @@ def _build_task_params(render_context: RenderContext, unittests_issue: str) -> d "unittests_log_path": render_context.script_execution_history.latest_unit_test_output_path, "file_tree": FixUnitTests._file_tree(render_context.build_folder), "relevant_files": FixUnitTests._relevant_files( - render_context.build_folder, render_context.frid_context.changed_files | context.changed_files + render_context.build_folder, + render_context.frid_context.changed_files | context.changed_files | conformance_fix_files, ), } - if context.previous_session_id: - task_params["previous_session_id"] = context.previous_session_id + if conformance_tests_fixes: + task_params["conformance_tests_fixes"] = conformance_tests_fixes + session = render_context.unit_tests_agent_session + if session.previous_session_id: + task_params["previous_session_id"] = session.previous_session_id return task_params @staticmethod diff --git a/render_machine/agent_tools.py b/render_machine/agent_tools.py index 53df2c70..e36727b2 100644 --- a/render_machine/agent_tools.py +++ b/render_machine/agent_tools.py @@ -46,13 +46,13 @@ def _readable(path: str, render_context: RenderContext) -> bool: return ( _within(path, _build_folder(render_context)) or _within(path, os.path.normpath(os.getcwd())) - or path in render_context.unit_tests_running_context.readable_log_paths + or path in render_context.unit_tests_agent_session.readable_log_paths ) def register_log_path(log_path: str, render_context: RenderContext) -> None: """Allow read_file/grep on a full test log the agent is pointed to.""" - render_context.unit_tests_running_context.readable_log_paths.add(os.path.normpath(os.path.abspath(log_path))) + render_context.unit_tests_agent_session.readable_log_paths.add(os.path.normpath(os.path.abspath(log_path))) def _writable(path: str, render_context: RenderContext) -> bool: diff --git a/render_machine/render_context.py b/render_machine/render_context.py index 75652953..62ba9331 100644 --- a/render_machine/render_context.py +++ b/render_machine/render_context.py @@ -19,6 +19,7 @@ FridContext, ScriptExecutionHistory, TestExecutionPhase, + UnitTestsAgentSession, UnitTestsRunningContext, ) @@ -188,6 +189,14 @@ def should_run_unit_tests(self) -> bool: def should_run_conformance_tests(self) -> bool: return self.conformance_tests_script is not None + @property + def unit_tests_agent_session(self) -> UnitTestsAgentSession: + """The agent session fixing the unit tests: per unit-test loop, except during the conformance + phase, where one session spans every loop (see UnitTestsAgentSession).""" + if self.conformance_tests_running_context is not None: + return self.conformance_tests_running_context.unit_tests_agent_session + return self.unit_tests_running_context.agent_session + def start_unittests_processing(self): self.unit_tests_running_context = UnitTestsRunningContext(fix_attempts=0) self.run_state.increment_unittest_batch_id() diff --git a/render_machine/render_types.py b/render_machine/render_types.py index 5455422e..269e6bb9 100644 --- a/render_machine/render_types.py +++ b/render_machine/render_types.py @@ -49,13 +49,16 @@ class FridContext: @dataclass -class UnitTestsRunningContext: - fix_attempts: int - changed_files: set[str] = field(default_factory=set) - # Server-side agent session fixing this FRID's unit tests. It spans every fix attempt of - # one unit-test loop (the context is recreated when the loop starts), so each new failure - # is fed back into the same conversation instead of a fresh, stateless call. - agent_session_id: Optional[str] = None +class UnitTestsAgentSession: + """Server-side agent session fixing a FRID's unit tests, and what it still has to be told. + + Owned by the unit-tests running context (one session per unit-test loop) in the implementation + and refactoring phases, and by the conformance tests running context during the conformance + phase, so there one session spans every unit-test loop - the agent then sees that the + conformance tests fixer keeps changing the code it adjusts (see RenderContext.unit_tests_agent_session). + """ + + session_id: Optional[str] = None # The submit_fix call the agent ended its last attempt with, answered with the next test # run's outcome, plus results of any tool calls made in the same turn as submit_fix. pending_submit_call_id: Optional[str] = None @@ -63,13 +66,33 @@ class UnitTestsRunningContext: # Session abandoned without a submission (turn budget used up, LLM failure); the next session # starts with a digest of what it tried. previous_session_id: Optional[str] = None + # Full test logs the agent was pointed to; readable by read_file/grep although outside the + # build folder. + readable_log_paths: set[str] = field(default_factory=set) + # How many of the conformance tests fixes (ConformanceTestsRunningContext.implementation_code_fixes) + # the session has already been shown. + conformance_fixes_handed_off: int = 0 + + def reset(self) -> None: + """Drop the session (keeping its id as the previous one) so the next failure starts a new one.""" + self.previous_session_id = self.session_id + self.session_id, self.pending_submit_call_id, self.pending_tool_results = None, None, [] + self.conformance_fixes_handed_off = 0 + + +@dataclass +class UnitTestsRunningContext: + fix_attempts: int + changed_files: set[str] = field(default_factory=set) + # The agent session of this loop; used only outside the conformance phase. + agent_session: UnitTestsAgentSession = field(default_factory=UnitTestsAgentSession) + # Whether the agent already made a fix attempt in this loop. A session that is still open when + # a loop starts had its last fix accepted, which is what the agent is told. + agent_used_in_this_loop: bool = False # Set when the agent's own run_unit_tests passed and no file changed since, so the harness # can accept the fix without running the suite again. verified_passing: bool = False verified_passing_log_path: Optional[str] = None - # Full test logs the agent was pointed to; readable by read_file/grep although outside the - # build folder. - readable_log_paths: set[str] = field(default_factory=set) # Results of read-only tool calls, keyed by call; cleared whenever a file changes. tool_result_cache: dict[str, str] = field(default_factory=dict) @@ -115,6 +138,8 @@ def __init__( # order. Each entry is {"hypothesis": str | None, "approach": str | None, "code_diff": {file: diff}}. # Handed to the unit tests fixer so it adjusts the unit tests instead of reverting these changes. self.implementation_code_fixes: list[dict] = [] + # The unit-test fixing agent session, shared by every unit-test loop of this conformance phase. + self.unit_tests_agent_session = UnitTestsAgentSession() def get_conformance_tests_json(self, module_name: str) -> dict: return self._conformance_tests_json[module_name] diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py index dd19fb29..efbe0fbd 100644 --- a/tests/test_agent_tools.py +++ b/tests/test_agent_tools.py @@ -6,9 +6,14 @@ import pytest from render_machine import agent_tools +from render_machine.render_context import RenderContext from render_machine.render_types import UnitTestsRunningContext +class FakeRenderContext(SimpleNamespace): + unit_tests_agent_session = RenderContext.unit_tests_agent_session + + @pytest.fixture def project(tmp_path, monkeypatch): build = tmp_path / "plain_modules" / "m" @@ -16,9 +21,10 @@ def project(tmp_path, monkeypatch): (build / "app.py").write_text("def add(a, b):\n return a - b\n\n\ndef sub(a, b):\n return a - b\n") (tmp_path / "outside.txt").write_text("outside\n") monkeypatch.chdir(tmp_path) - render_context = SimpleNamespace( + render_context = FakeRenderContext( build_folder=str(build), unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), + conformance_tests_running_context=None, unittests_script=None, test_script_timeout=None, stop_event=None, diff --git a/tests/test_fix_unit_tests_action.py b/tests/test_fix_unit_tests_action.py index 4deda172..54fc1de8 100644 --- a/tests/test_fix_unit_tests_action.py +++ b/tests/test_fix_unit_tests_action.py @@ -7,9 +7,16 @@ import plain_spec from render_machine.actions.fix_unit_tests import MAX_AGENT_TURNS_PER_ATTEMPT, MAX_RELEVANT_FILES_CHARS, FixUnitTests from render_machine.actions.run_unit_tests import RunUnitTests +from render_machine.render_context import RenderContext from render_machine.render_types import ScriptExecutionHistory, UnitTestsRunningContext +class FakeRenderContext(SimpleNamespace): + """A render context with just the attributes FixUnitTests uses, and the real session lookup.""" + + unit_tests_agent_session = RenderContext.unit_tests_agent_session + + class FakeAPI: def __init__(self, responses): self.responses = list(responses) @@ -41,7 +48,7 @@ def render_context(tmp_path, monkeypatch): plain_spec.FUNCTIONAL_REQUIREMENTS: ["- Old feature.", "- New feature."], } monkeypatch.setattr(plain_spec, "get_specifications_for_frid", lambda tree, frid: (specifications, None)) - return SimpleNamespace( + return FakeRenderContext( codeplain_api=None, build_folder=str(build), module_name="m", @@ -52,6 +59,7 @@ def render_context(tmp_path, monkeypatch): stop_event=None, frid_context=SimpleNamespace(frid="2", linked_resources={"schema.json": "{}"}, changed_files={"a.py"}), unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), + conformance_tests_running_context=None, script_execution_history=ScriptExecutionHistory(), get_required_modules_functionalities=lambda: {"base": ["- Base feature."]}, ) @@ -91,17 +99,21 @@ def test_first_attempt_starts_session_runs_tools_and_stops_at_submit_fix(render_ assert open(render_context.build_folder + "/a.py").read() == "x = 2\n" context = render_context.unit_tests_running_context - assert context.agent_session_id == "s1" - assert context.pending_submit_call_id == "c3" - assert [r["call_id"] for r in context.pending_tool_results] == ["c2"] + session = context.agent_session + assert session.session_id == "s1" + assert session.pending_submit_call_id == "c3" + assert [r["call_id"] for r in session.pending_tool_results] == ["c2"] assert context.changed_files == {"a.py"} + assert "conformance_tests_fixes" not in task_params def test_second_attempt_continues_session_answering_submit_fix(render_context): context = render_context.unit_tests_running_context - context.agent_session_id = "s1" - context.pending_submit_call_id = "c3" - context.pending_tool_results = [{"call_id": "c2", "output": "Edited"}] + context.agent_used_in_this_loop = True + session = context.agent_session + session.session_id = "s1" + session.pending_submit_call_id = "c3" + session.pending_tool_results = [{"call_id": "c2", "output": "Edited"}] api = FakeAPI([_tool_calls({"id": "c4", "name": "submit_fix", "args": {"changes_made": "again"}})]) render_context.codeplain_api = api @@ -113,8 +125,9 @@ def test_second_attempt_continues_session_answering_submit_fix(render_context): assert tool_results[0] == {"call_id": "c2", "output": "Edited"} assert tool_results[1]["call_id"] == "c3" and "still fail" in tool_results[1]["output"] assert tool_results[1]["test_output"] == "FAILED test_b" - assert context.agent_session_id == "s1" and context.pending_submit_call_id == "c4" - assert context.pending_tool_results == [] + assert "conformance_tests_fixes" not in tool_results[1] + assert session.session_id == "s1" and session.pending_submit_call_id == "c4" + assert session.pending_tool_results == [] @pytest.mark.parametrize( @@ -128,8 +141,8 @@ def test_session_ending_without_submission_resets_the_session(render_context, fi render_context.codeplain_api = FakeAPI([final_response]) outcome, _ = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME - context = render_context.unit_tests_running_context - assert context.agent_session_id is None and context.pending_submit_call_id is None + session = render_context.unit_tests_running_context.agent_session + assert session.session_id is None and session.pending_submit_call_id is None def test_turn_cap_per_attempt_resets_the_session(render_context): @@ -137,7 +150,7 @@ def test_turn_cap_per_attempt_resets_the_session(render_context): render_context.codeplain_api = FakeAPI([_tool_calls(call)] * (MAX_AGENT_TURNS_PER_ATTEMPT + 1)) FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) assert len(render_context.codeplain_api.calls) == MAX_AGENT_TURNS_PER_ATTEMPT + 1 - assert render_context.unit_tests_running_context.agent_session_id is None + assert render_context.unit_tests_running_context.agent_session.session_id is None def test_missing_issue_is_an_internal_error(render_context): @@ -164,7 +177,7 @@ def test_first_turn_is_seeded_with_file_tree_relevant_files_and_log_path(render_ assert task_params["unittests_log_path"] == str(log) assert "previous_session_id" not in task_params # the agent may grep the full log although it is outside the build folder and project root - assert str(log) in render_context.unit_tests_running_context.readable_log_paths + assert str(log) in render_context.unit_tests_running_context.agent_session.readable_log_paths def test_relevant_files_stay_within_budget(tmp_path): @@ -177,7 +190,7 @@ def test_new_session_after_abandoned_one_references_it(render_context): call = {"id": "c", "name": "ls_files", "args": {}} render_context.codeplain_api = FakeAPI([_tool_calls(call)] * (MAX_AGENT_TURNS_PER_ATTEMPT + 1)) FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) - assert render_context.unit_tests_running_context.previous_session_id == "s1" + assert render_context.unit_tests_running_context.agent_session.previous_session_id == "s1" api = FakeAPI([{"session_id": "s2", "status": "completed", "result": "done"}]) render_context.codeplain_api = api diff --git a/tests/test_fix_unit_tests_conformance_context.py b/tests/test_fix_unit_tests_conformance_context.py index a2fe3a1e..7d29f07d 100644 --- a/tests/test_fix_unit_tests_conformance_context.py +++ b/tests/test_fix_unit_tests_conformance_context.py @@ -12,25 +12,44 @@ import plain_spec from memory_management import MemoryManager -from render_machine.actions import fix_unit_tests as fix_unit_tests_module from render_machine.actions.fix_conformance_test import FixConformanceTest from render_machine.actions.fix_unit_tests import FixUnitTests from render_machine.implementation_code_helpers import ImplementationCodeHelpers -from render_machine.render_types import ConformanceTestsRunningContext, UnitTestsRunningContext +from render_machine.render_context import RenderContext +from render_machine.render_types import ConformanceTestsRunningContext, ScriptExecutionHistory, UnitTestsRunningContext class FakeCodeplainAPI: - def __init__(self, conformance_fix_response=None, unittests_fix_response=None): + """Conformance fixes return a scripted response; every agent session submits a fix on its first turn.""" + + def __init__(self, conformance_fix_response=None): self.conformance_fix_response = conformance_fix_response - self.unittests_fix_response = unittests_fix_response or {} - self.unittests_fix_calls = [] + self.agent_calls = [] + self.sessions_started = 0 def fix_conformance_tests_issue(self, *args, **kwargs): return self.conformance_fix_response - def fix_unittests_issue(self, *args, **kwargs): - self.unittests_fix_calls.append(kwargs) - return self.unittests_fix_response + def agent_start(self, task_type, task_params, frid, module_name, run_state): + self.sessions_started += 1 + self.agent_calls.append(("start", task_params)) + return self._submit(f"s{self.sessions_started}") + + def agent_continue(self, session_id, tool_results, frid, module_name, run_state): + self.agent_calls.append(("continue", session_id, tool_results)) + return self._submit(session_id) + + def _submit(self, session_id): + call_id = f"submit-{len(self.agent_calls)}" + return { + "session_id": session_id, + "status": "tool_calls", + "calls": [{"id": call_id, "name": "submit_fix", "args": {"changes_made": "fixed"}}], + } + + +class FakeRenderContext(SimpleNamespace): + unit_tests_agent_session = RenderContext.unit_tests_agent_session class FakeConformanceTests: @@ -59,7 +78,11 @@ def memory_folder(): def isolate_from_git_and_console(monkeypatch): monkeypatch.setattr(ImplementationCodeHelpers, "get_code_diff", staticmethod(lambda *args: {})) monkeypatch.setattr(plain_spec, "collect_linked_resources", lambda *args: None) - monkeypatch.setattr(fix_unit_tests_module.render_utils, "print_inputs", lambda *args: None) + monkeypatch.setattr( + plain_spec, + "get_specifications_for_frid", + lambda tree, frid: ({plain_spec.FUNCTIONAL_REQUIREMENTS: ["- Add numbers."]}, None), + ) def make_conformance_context(): @@ -76,7 +99,7 @@ def make_conformance_context(): def make_render_context(api, build_folder, memory_folder, conformance_tests_running_context): - return SimpleNamespace( + return FakeRenderContext( codeplain_api=api, build_folder=build_folder, memory_manager=MemoryManager(api, memory_folder), @@ -86,9 +109,11 @@ def make_render_context(api, build_folder, memory_folder, conformance_tests_runn plain_source_tree={}, module_name="mod", required_modules=None, - frid_context=SimpleNamespace(frid="1", linked_resources={}), + frid_context=SimpleNamespace(frid="1", linked_resources={}, changed_files=set()), get_required_modules_functionalities=lambda: {}, run_state=SimpleNamespace(render_id="test-render-id", unittest_batch_id=1), + unittests_script=None, + script_execution_history=ScriptExecutionHistory(), ) @@ -191,34 +216,104 @@ def test_implementation_fix_with_no_files_is_not_remembered(build_folder, memory assert ctx.implementation_code_fixes == [] -def run_unit_tests_fix(render_context): - return FixUnitTests().execute(render_context, {"previous_unittests_issue": "1 failed"}) +def run_unit_tests_fix(render_context, issue="1 failed"): + return FixUnitTests().execute(render_context, {"previous_unittests_issue": issue}) + + +def start_new_unit_test_loop(render_context): + """What RenderContext.start_unittests_processing does when the unit tests are run again.""" + render_context.unit_tests_running_context = UnitTestsRunningContext(fix_attempts=0) + + +def conformance_fix(number): + return { + "hypothesis": f"hypothesis {number}", + "approach": f"approach {number}", + "code_diff": {"app.py": f"+{number}"}, + } -def test_unit_tests_fix_forwards_conformance_fixes(build_folder, memory_folder): - api = FakeCodeplainAPI(unittests_fix_response={"test_app.py": "def test_add(): pass\n"}) +def test_first_unit_test_loop_of_conformance_phase_seeds_the_session_with_the_fixes(build_folder, memory_folder): + api = FakeCodeplainAPI() ctx = make_conformance_context() - ctx.implementation_code_fixes.append( - {"hypothesis": "off by one", "approach": "add one", "code_diff": {"app.py": "+ return a + b + 1"}} - ) + ctx.implementation_code_fixes.append(conformance_fix(1)) render_context = make_render_context(api, build_folder, memory_folder, ctx) outcome, _ = run_unit_tests_fix(render_context) assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME - assert len(api.unittests_fix_calls) == 1 - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] == ctx.implementation_code_fixes + kind, task_params = api.agent_calls[0] + assert kind == "start" + assert task_params["conformance_tests_fixes"] == ctx.implementation_code_fixes # The forwarded list is a copy, so later conformance fixes do not mutate what was sent. - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is not ctx.implementation_code_fixes + assert task_params["conformance_tests_fixes"] is not ctx.implementation_code_fixes + # The file the conformance fix changed is seeded although the FRID did not change it. + assert "app.py" in task_params["relevant_files"] + assert ctx.unit_tests_agent_session.session_id == "s1" -def test_unit_tests_fix_outside_conformance_phase_sends_no_fixes(build_folder, memory_folder): +def test_next_unit_test_loop_of_conformance_phase_continues_the_session_with_only_new_fixes( + build_folder, memory_folder +): api = FakeCodeplainAPI() - render_context = make_render_context(api, build_folder, memory_folder, None) + ctx = make_conformance_context() + ctx.implementation_code_fixes.append(conformance_fix(1)) + render_context = make_render_context(api, build_folder, memory_folder, ctx) + run_unit_tests_fix(render_context) + + # The fix was accepted; the conformance tests fixer changes the code again and the unit tests fail again. + ctx.implementation_code_fixes.append(conformance_fix(2)) + start_new_unit_test_loop(render_context) + run_unit_tests_fix(render_context, issue="2 failed") + + assert api.sessions_started == 1 + kind, session_id, tool_results = api.agent_calls[1] + assert (kind, session_id) == ("continue", "s1") + submit_answer = tool_results[-1] + assert submit_answer["call_id"] == "submit-1" + assert submit_answer["output"].startswith("Your fix was accepted: the unit tests passed.") + assert "Conformance Tests Fix below" in submit_answer["output"] + assert submit_answer["conformance_tests_fixes"] == [conformance_fix(2)] + assert submit_answer["test_output"] == "2 failed" + +def test_retry_within_a_unit_test_loop_says_the_fix_did_not_work(build_folder, memory_folder): + api = FakeCodeplainAPI() + ctx = make_conformance_context() + ctx.implementation_code_fixes.append(conformance_fix(1)) + render_context = make_render_context(api, build_folder, memory_folder, ctx) + run_unit_tests_fix(render_context) + + run_unit_tests_fix(render_context, issue="still failing") + + submit_answer = api.agent_calls[1][2][-1] + assert "still fail" in submit_answer["output"] + # Fix 1 was already shown to the session, so it is not sent again. + assert "conformance_tests_fixes" not in submit_answer + + +def test_new_conformance_phase_starts_a_new_session(build_folder, memory_folder): + api = FakeCodeplainAPI() + render_context = make_render_context(api, build_folder, memory_folder, make_conformance_context()) + run_unit_tests_fix(render_context) + + # E.g. the functionality is re-rendered from scratch: the conformance tests running context is recreated. + render_context.conformance_tests_running_context = make_conformance_context() + start_new_unit_test_loop(render_context) + run_unit_tests_fix(render_context) + + assert [call[0] for call in api.agent_calls] == ["start", "start"] + + +def test_unit_tests_outside_conformance_phase_get_no_fixes_and_a_session_per_loop(build_folder, memory_folder): + api = FakeCodeplainAPI() + render_context = make_render_context(api, build_folder, memory_folder, None) + run_unit_tests_fix(render_context) + start_new_unit_test_loop(render_context) run_unit_tests_fix(render_context) - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is None + assert [call[0] for call in api.agent_calls] == ["start", "start"] + assert all("conformance_tests_fixes" not in call[1] for call in api.agent_calls) def test_unit_tests_fix_in_conformance_phase_without_implementation_changes_sends_no_fixes(build_folder, memory_folder): @@ -227,4 +322,4 @@ def test_unit_tests_fix_in_conformance_phase_without_implementation_changes_send run_unit_tests_fix(render_context) - assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is None + assert "conformance_tests_fixes" not in api.agent_calls[0][1] From 1cdb8ec850f1f5336a0027fc7107ab88a7d4405c Mon Sep 17 00:00:00 2001 From: VitjanZ Date: Tue, 29 Sep 2026 15:35:44 +0200 Subject: [PATCH 4/5] Start a new agent session when the old one has expired Agent sessions expire on the server (Redis TTL). Continuing an expired one returned 404, which failed the render; FixUnitTests now starts a new session instead. Other errors are raised as before. --- render_machine/actions/fix_unit_tests.py | 51 +++++++++++++++++------- tests/test_fix_unit_tests_action.py | 31 +++++++++++++- 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/render_machine/actions/fix_unit_tests.py b/render_machine/actions/fix_unit_tests.py index f6485059..4575b9e2 100644 --- a/render_machine/actions/fix_unit_tests.py +++ b/render_machine/actions/fix_unit_tests.py @@ -1,6 +1,9 @@ import os +from http import HTTPStatus from typing import Any +import requests + import file_utils import plain_spec from plain2code_console import console @@ -61,19 +64,8 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | "change(s) made to fix the conformance tests." ) - if session.session_id is None: - console.info("Starting an agent session to fix the unit tests.") - # Cached read results point at earlier turns, which a new session does not have. - context.tool_result_cache.clear() - response = api.agent_start( - TASK_TYPE, - self._build_task_params(render_context, unittests_issue, conformance_tests_fixes), - frid, - module_name, - render_context.run_state, - ) - session.session_id = response["session_id"] - else: + response = None + if session.session_id is not None: if context.agent_used_in_this_loop: console.info(f"Continuing agent session {session.session_id} with the new unit tests failure.") output = "The fix was applied, but the unit tests still fail." @@ -98,7 +90,20 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | submit_result["conformance_tests_fixes"] = new_conformance_tests_fixes tool_results = session.pending_tool_results + [submit_result] session.pending_tool_results, session.pending_submit_call_id = [], None - response = api.agent_continue(session.session_id, tool_results, frid, module_name, render_context.run_state) + response = self._continue_session(render_context, session.session_id, tool_results) + if response is None: + console.info("Starting an agent session to fix the unit tests.") + # Cached read results point at earlier turns, which a new session does not have. + context.tool_result_cache.clear() + response = api.agent_start( + TASK_TYPE, + self._build_task_params(render_context, unittests_issue, conformance_tests_fixes), + frid, + module_name, + render_context.run_state, + ) + session.session_id = response["session_id"] + assert session.session_id is not None session.conformance_fixes_handed_off = len(conformance_tests_fixes) context.agent_used_in_this_loop = True @@ -146,6 +151,24 @@ def _get_conformance_tests_fixes(render_context: RenderContext) -> list[dict]: return [] return list(getattr(conformance_tests_running_context, "implementation_code_fixes", None) or []) + @staticmethod + def _continue_session(render_context: RenderContext, session_id: str, tool_results: list[dict]) -> dict | None: + """Continue the session; None if the server no longer has it (expired), so a new one is started.""" + try: + return render_context.codeplain_api.agent_continue( + session_id, + tool_results, + render_context.frid_context.frid, + render_context.module_name, + render_context.run_state, + ) + except requests.exceptions.HTTPError as e: + if e.response is None or e.response.status_code != HTTPStatus.NOT_FOUND: + raise + console.warning(f"Agent session {session_id} has expired; starting a new one.") + render_context.unit_tests_agent_session.reset() + return None + @staticmethod def _build_task_params( render_context: RenderContext, unittests_issue: str, conformance_tests_fixes: list[dict] diff --git a/tests/test_fix_unit_tests_action.py b/tests/test_fix_unit_tests_action.py index 54fc1de8..27b9bea5 100644 --- a/tests/test_fix_unit_tests_action.py +++ b/tests/test_fix_unit_tests_action.py @@ -3,6 +3,7 @@ from types import SimpleNamespace import pytest +import requests import plain_spec from render_machine.actions.fix_unit_tests import MAX_AGENT_TURNS_PER_ATTEMPT, MAX_RELEVANT_FILES_CHARS, FixUnitTests @@ -28,7 +29,10 @@ def agent_start(self, task_type, task_params, frid, module_name, run_state): def agent_continue(self, session_id, tool_results, frid, module_name, run_state): self.calls.append(("continue", session_id, tool_results, frid, module_name)) - return self.responses.pop(0) + response = self.responses.pop(0) + if isinstance(response, Exception): + raise response + return response def _tool_calls(*calls): @@ -208,3 +212,28 @@ def test_run_unit_tests_action_skips_the_suite_after_a_verified_agent_run(render assert RunUnitTests().execute(render_context, None) == (RunUnitTests.SUCCESSFUL_OUTCOME, None) assert context.verified_passing is False assert render_context.script_execution_history.latest_unit_test_output_path == "/logs/pass.log" + + +def test_expired_session_is_replaced_by_a_new_one(render_context): + context = render_context.unit_tests_running_context + context.agent_used_in_this_loop = True + session = context.agent_session + session.session_id, session.pending_submit_call_id = "expired", "c3" + not_found = requests.exceptions.HTTPError(response=SimpleNamespace(status_code=404)) + api = FakeAPI([not_found, {"session_id": "s2", "status": "completed", "result": "done"}]) + render_context.codeplain_api = api + + outcome, _ = FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED test_b"}) + + assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME + assert [call[0] for call in api.calls] == ["continue", "start"] + assert api.calls[1][2]["unittests_issue"] == "FAILED test_b" + + +def test_other_http_errors_on_continue_are_not_swallowed(render_context): + context = render_context.unit_tests_running_context + context.agent_used_in_this_loop = True + context.agent_session.session_id, context.agent_session.pending_submit_call_id = "s1", "c3" + render_context.codeplain_api = FakeAPI([requests.exceptions.HTTPError(response=SimpleNamespace(status_code=500))]) + with pytest.raises(requests.exceptions.HTTPError): + FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) From 6a75e41ce333b6dc1c8337c7eac2483b8cc49739 Mon Sep 17 00:00:00 2001 From: VitjanZ Date: Fri, 2 Oct 2026 09:00:56 +0200 Subject: [PATCH 5/5] Confine agent reads to the build folder read_file, grep and ls_files also accepted anything under the directory codeplain runs from, which exposes the specs, other modules and any .env in the project root; whatever the agent reads goes to the server and the LLM. Reads are now limited to the build folder plus the test logs the agent is pointed to. --- render_machine/agent_tools.py | 12 +++++------- tests/test_agent_tools.py | 10 +++++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/render_machine/agent_tools.py b/render_machine/agent_tools.py index e36727b2..14c7309c 100644 --- a/render_machine/agent_tools.py +++ b/render_machine/agent_tools.py @@ -2,8 +2,8 @@ The server declares the tools to the LLM (codeplain-api: src/agent/tools.py) and forwards the model's calls; this module executes them against the local build folder and returns -plain-text results. Relative paths resolve against the build folder. Reads are allowed in the -build folder and the project root (the CWD); writes only inside the build folder. +plain-text results. Relative paths resolve against the build folder. Reads are allowed only in the +build folder and the full test logs the agent was pointed to; writes only inside the build folder. """ import glob @@ -45,7 +45,6 @@ def _within(path: str, folder: str) -> bool: def _readable(path: str, render_context: RenderContext) -> bool: return ( _within(path, _build_folder(render_context)) - or _within(path, os.path.normpath(os.getcwd())) or path in render_context.unit_tests_agent_session.readable_log_paths ) @@ -82,7 +81,7 @@ def _track_change(full_path: str, render_context: RenderContext) -> None: def read_file(args: dict, render_context: RenderContext) -> str: full_path = _resolve(args.get("file_path", ""), render_context) if not _readable(full_path, render_context): - return f"Error: read access denied for '{full_path}' (readable: build folder and project root)." + return f"Error: read access denied for '{full_path}' (readable: build folder and test logs)." if not os.path.isfile(full_path): return f"Error: file not found: '{full_path}'." with open(full_path, "r", encoding="utf-8", errors="replace") as f: @@ -112,9 +111,8 @@ def grep(args: dict, render_context: RenderContext) -> str: if not os.path.exists(target): return f"Error: path not found: '{target}'." # Run from the build folder so matches inside it come back as build-relative paths, which - # is the form the other tools accept. - build_folder = _build_folder(render_context) - cwd = build_folder if _within(target, build_folder) else os.getcwd() + # is the form the other tools accept; a test log outside it is passed by absolute path. + cwd = _build_folder(render_context) options = [f"--exclude-dir={d}" for d in GREP_EXCLUDED_DIRS] context_lines = min(max(int(args.get("context_lines") or 0), 0), MAX_GREP_CONTEXT_LINES) if context_lines: diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py index efbe0fbd..e4e8ed4a 100644 --- a/tests/test_agent_tools.py +++ b/tests/test_agent_tools.py @@ -38,8 +38,12 @@ def test_read_file_resolves_relative_to_build_folder_with_paging(project): assert "use offset=3 to continue" in out -def test_read_allows_project_root_but_not_elsewhere(project): - assert "outside" in agent_tools.read_file({"file_path": str(project.root / "outside.txt")}, project.rc) +def test_read_is_confined_to_build_folder(project): + outside = str(project.root / "outside.txt") + assert agent_tools.read_file({"file_path": outside}, project.rc).startswith("Error: read access denied") + assert agent_tools.read_file({"file_path": "../../outside.txt"}, project.rc).startswith("Error: read access denied") + assert agent_tools.grep({"pattern": "outside", "file_path": outside}, project.rc).startswith("Error: read access") + assert agent_tools.ls_files({"pattern": str(project.root)}, project.rc).startswith("Error: read access denied") assert agent_tools.read_file({"file_path": "/etc/hosts"}, project.rc).startswith("Error: read access denied") assert agent_tools.read_file({"file_path": "missing.py"}, project.rc).startswith("Error: file not found") @@ -118,7 +122,7 @@ def test_run_unit_tests_reports_pass_and_failure(project, monkeypatch): assert failure["output"].startswith(f"Unit tests failed (exit code 1). Full log: {log}") # raw output goes to the server for condensing, not truncated here assert failure["test_output"] == "FAILED test_x" - # the full log is outside the build folder and project root but greppable + # the full log is outside the build folder but greppable assert agent_tools.grep({"pattern": "Caused by", "file_path": str(log)}, project.rc).endswith("Caused by: boom")