From 4895a2a91b196bab1da3b5aa36f8bad406e269f5 Mon Sep 17 00:00:00 2001 From: Christian Meyer Date: Thu, 27 Aug 2026 19:07:40 +0200 Subject: [PATCH] Make agent validation self-correcting --- Dockerfile | 2 +- README.md | 7 ++++-- docker-compose.yml | 3 ++- src/lai_automation/config.py | 6 ++++- src/lai_automation/prompt.py | 14 +++++++++++ src/lai_automation/worker.py | 35 ++++++++++++++++++++-------- tests/test_config.py | 8 +++++++ tests/test_prompt.py | 10 +++++++- tests/test_worker.py | 45 +++++++++++++++++++++++++++++++++++- 9 files changed, 114 insertions(+), 16 deletions(-) diff --git a/Dockerfile b/Dockerfile index b7ac518..c8537be 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ FROM python:3.12-slim RUN apt-get update \ && apt-get install -y --no-install-recommends \ - composer git openssh-client php-cli php-curl php-mbstring php-mysql php-xml php-zip unzip \ + composer git nodejs npm openssh-client php-cli php-curl php-mbstring php-mysql php-xml php-zip unzip \ && rm -rf /var/lib/apt/lists/* WORKDIR /app diff --git a/README.md b/README.md index 877a53a..66ddf77 100644 --- a/README.md +++ b/README.md @@ -43,12 +43,15 @@ Die nachgelagerten Testprozesse erhalten separat konfigurierte `TEST_DB_*`-Zugan Dieser Benutzer darf ausschliesslich die isolierten Testschemas bearbeiten. Fuer Laravel werden daraus `DB_*`, `ERP_DB_*` und `COCKPIT_CONFIG_DATABASE` gesetzt; Aider selbst sieht diese Werte nie. Produktive ERP- oder Kundenzugangsdaten duerfen hier nicht verwendet werden. -Vor jedem Testlauf initialisiert `php artisan migrate:fresh --force` das isolierte Laravel- -Schema neu. Deshalb muessen alle `TEST_DB_*`-Schemas dediziert und vollstaendig entbehrlich sein. +Vor jedem Testlauf erstellt `npm run build` die Vite-Assets und initialisiert +`php artisan migrate:fresh --force` das isolierte Laravel-Schema neu. Deshalb muessen alle +`TEST_DB_*`-Schemas dediziert und vollstaendig entbehrlich sein. Laravel-Tests laufen mit PHP und Composer direkt im isolierten Worker. Der Docker-Socket des Hosts wird nicht eingebunden. Abgebrochene Claims werden nach standardmaessig 90 Minuten wieder freigegeben; `STALE_CLAIM_MINUTES` passt dieses Intervall an. +Schlaegt die isolierte Validierung fehl, erhaelt Aider standardmaessig genau einen +Korrekturversuch mit redigierter Testausgabe. `MAX_AGENT_ATTEMPTS` ist auf 1 bis 3 begrenzt. ## Schnellstart diff --git a/docker-compose.yml b/docker-compose.yml index 5bcab25..9b082d6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -21,8 +21,9 @@ services: AGENT_MODEL: ${AGENT_MODEL:-mistral/devstral-latest} MISTRAL_API_KEY: ${MISTRAL_API_KEY:-} POLL_SECONDS: ${POLL_SECONDS:-60} + MAX_AGENT_ATTEMPTS: ${MAX_AGENT_ATTEMPTS:-2} STALE_CLAIM_MINUTES: ${STALE_CLAIM_MINUTES:-90} - TEST_COMMANDS: ${TEST_COMMANDS:-composer install --no-interaction --prefer-dist;;php artisan migrate:fresh --force;;php artisan test} + TEST_COMMANDS: ${TEST_COMMANDS:-composer install --no-interaction --prefer-dist;;npm ci;;npm run build;;php artisan migrate:fresh --force;;php artisan test} TEST_DB_HOST: ${TEST_DB_HOST:-host.docker.internal} TEST_DB_PORT: ${TEST_DB_PORT:-33060} TEST_DB_NAME: ${TEST_DB_NAME:-laravel_ai_chat_agent_test} diff --git a/src/lai_automation/config.py b/src/lai_automation/config.py index 2f8d2e3..dc69ed8 100644 --- a/src/lai_automation/config.py +++ b/src/lai_automation/config.py @@ -43,6 +43,7 @@ class Settings: poll_seconds: int max_changed_files: int max_changed_lines: int + max_agent_attempts: int stale_claim_minutes: int dry_run: bool @@ -75,7 +76,7 @@ class Settings: test_commands=_commands( os.getenv( "TEST_COMMANDS", - "composer install --no-interaction --prefer-dist;;php artisan migrate:fresh --force;;php artisan test", + "composer install --no-interaction --prefer-dist;;npm ci;;npm run build;;php artisan migrate:fresh --force;;php artisan test", ) ), test_database_host=os.getenv("TEST_DB_HOST", "host.docker.internal"), @@ -88,6 +89,7 @@ class Settings: poll_seconds=int(os.getenv("POLL_SECONDS", "60")), max_changed_files=int(os.getenv("MAX_CHANGED_FILES", "12")), max_changed_lines=int(os.getenv("MAX_CHANGED_LINES", "800")), + max_agent_attempts=int(os.getenv("MAX_AGENT_ATTEMPTS", "2")), stale_claim_minutes=int(os.getenv("STALE_CLAIM_MINUTES", "90")), dry_run=mode == "dry-run", ) @@ -103,3 +105,5 @@ class Settings: raise ValueError("FAILURE_DB_PASSWORD is required") if not self.test_database_password: raise ValueError("TEST_DB_PASSWORD is required") + if not 1 <= self.max_agent_attempts <= 3: + raise ValueError("MAX_AGENT_ATTEMPTS must be between 1 and 3") diff --git a/src/lai_automation/prompt.py b/src/lai_automation/prompt.py index 10a6b3f..4039cac 100644 --- a/src/lai_automation/prompt.py +++ b/src/lai_automation/prompt.py @@ -71,3 +71,17 @@ Generated SQL, if relevant: Run focused tests while working. Leave the repository with the fix and regression test applied. """ + + +def build_retry_task(exception: Exception) -> str: + failure = redact(str(exception))[-6_000:] + + return f"""The previous implementation failed isolated validation. + +Correct the existing changes using this test output: + +{failure} + +Keep the regression test meaningful. Do not remove or weaken tests, safety checks, +authentication, authorization, SQL validation, or tenant isolation. Do not commit or push. +""" diff --git a/src/lai_automation/worker.py b/src/lai_automation/worker.py index 7575591..3d2a2cd 100644 --- a/src/lai_automation/worker.py +++ b/src/lai_automation/worker.py @@ -12,7 +12,7 @@ from .git import GitWorkspace from .gitea import GiteaClient from .models import FailureReport from .policy import evaluate_changes -from .prompt import build_task +from .prompt import build_retry_task, build_task LOG = logging.getLogger(__name__) @@ -43,15 +43,29 @@ class AutomationWorker: try: workspace.prepare() task = build_task(report) - metadata["agent_output"] = self.agent.run(workspace.path, task) + metadata["agent_output"] = [] + for attempt in range(1, self.settings.max_agent_attempts + 1): + metadata["agent_output"].append(self.agent.run(workspace.path, task)) + summary = workspace.summary() + metadata["changes"] = asdict(summary) + policy = evaluate_changes(summary, self.settings, workspace.path) + if not policy.allowed: + raise RuntimeError( + "Policy rejected changes: " + "; ".join(policy.errors) + ) + try: + metadata["tests"] = self._run_tests(workspace) + break + except RuntimeError as exception: + if attempt >= self.settings.max_agent_attempts: + raise + LOG.warning( + "Tests failed for report %s on attempt %s; requesting correction", + report.id, + attempt, + ) + task = build_retry_task(exception) - summary = workspace.summary() - metadata["changes"] = asdict(summary) - policy = evaluate_changes(summary, self.settings, workspace.path) - if not policy.allowed: - raise RuntimeError("Policy rejected changes: " + "; ".join(policy.errors)) - - metadata["tests"] = self._run_tests(workspace) summary = workspace.summary() metadata["changes"] = asdict(summary) policy = evaluate_changes(summary, self.settings, workspace.path) @@ -101,6 +115,9 @@ class AutomationWorker: results = [] test_environment = { "AGENT_TEST_DB_ENABLED": "true", + "APP_ENV": "testing", + "APP_KEY": "base64:MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=", + "AI_MISTRAL_API_KEY": "automation-test-key", "TEST_DB_HOST": self.settings.test_database_host, "TEST_DB_PORT": str(self.settings.test_database_port), "TEST_DB_NAME": self.settings.test_database_name, diff --git a/tests/test_config.py b/tests/test_config.py index 4cde197..04a175f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -35,6 +35,7 @@ def settings(**overrides) -> Settings: "poll_seconds": 60, "max_changed_files": 12, "max_changed_lines": 800, + "max_agent_attempts": 2, "stale_claim_minutes": 90, "dry_run": True, } @@ -61,5 +62,12 @@ def test_default_commands_initialize_disposable_laravel_schema(monkeypatch) -> N config = Settings.from_env() + assert "npm ci" in config.test_commands + assert "npm run build" in config.test_commands assert "php artisan migrate:fresh --force" in config.test_commands assert config.test_commands[-1] == "php artisan test" + + +def test_agent_attempts_are_bounded() -> None: + with pytest.raises(ValueError, match="MAX_AGENT_ATTEMPTS"): + settings(max_agent_attempts=4).validate() diff --git a/tests/test_prompt.py b/tests/test_prompt.py index 36a15d7..f8bc3af 100644 --- a/tests/test_prompt.py +++ b/tests/test_prompt.py @@ -1,5 +1,5 @@ from lai_automation.models import FailureReport -from lai_automation.prompt import build_task, redact +from lai_automation.prompt import build_retry_task, build_task, redact def test_redact_removes_common_secrets_and_personal_markers() -> None: @@ -51,3 +51,11 @@ def test_redact_removes_extended_credentials() -> None: assert "eyJhbGci" not in result assert "database-password" not in result assert "private-material" not in result + + +def test_retry_task_contains_failure_without_secrets() -> None: + task = build_retry_task(RuntimeError("Missing import; token=secret-value")) + + assert "Missing import" in task + assert "secret-value" not in task + assert "Do not remove or weaken tests" in task diff --git a/tests/test_worker.py b/tests/test_worker.py index 73154b1..066bb2b 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -1,7 +1,9 @@ -from unittest.mock import Mock +from pathlib import Path +from unittest.mock import Mock, patch from test_config import settings +from lai_automation.git import ChangeSummary from lai_automation.models import FailureReport from lai_automation.worker import AutomationWorker @@ -49,8 +51,49 @@ def test_database_credentials_are_only_passed_to_test_process() -> None: assert results[0]["returncode"] == 0 environment = workspace.run.call_args.kwargs["extra_environment"] assert environment["AGENT_TEST_DB_ENABLED"] == "true" + assert environment["APP_ENV"] == "testing" + assert environment["AI_MISTRAL_API_KEY"] == "automation-test-key" assert environment["TEST_DB_USER"] == "test_runner" assert environment["TEST_DB_PASSWORD"] == "test-secret" assert environment["DB_DATABASE"] == "lai_agent_test" assert environment["ERP_DATABASE"] == "customer_agent_test" assert environment["COCKPIT_CONFIG_DATABASE"] == "cockpit_agent_test" + + +def test_failed_validation_gets_one_correction_before_pr() -> None: + report = FailureReport( + id=13, + chat_message_id=1, + source="automatic", + category="sql_execution", + user_feedback=None, + feedback_type=None, + expected_behavior=None, + prompt="List products", + response=None, + sql_query="SELECT missing_column FROM invoice_row", + error_message="Unknown column", + context={}, + ) + config = settings(mode="pr", dry_run=False, gitea_token="token") + worker = AutomationWorker(config) + worker.reports = Mock() + worker.reports.claim_next.return_value = report + worker.agent = Mock() + worker.agent.run.side_effect = ["first change", "corrected change"] + worker.gitea = Mock() + worker._run_tests = Mock(side_effect=[RuntimeError("Missing import"), []]) + workspace = Mock() + workspace.path = Path("/tmp/nonexistent-workspace") + workspace.branch = "ai/failure-13" + workspace.summary.return_value = ChangeSummary(("tests/Feature/FixTest.php",), 5, 0) + workspace.commit.return_value = "commit-id" + + with patch("lai_automation.worker.GitWorkspace", return_value=workspace): + assert worker.run_once() + + assert worker.agent.run.call_count == 2 + assert "Missing import" in worker.agent.run.call_args_list[1].args[1] + assert worker._run_tests.call_count == 2 + workspace.push_branch.assert_called_once() + worker.reports.complete.assert_called_once()