Make agent validation self-correcting
This commit is contained in:
@@ -2,7 +2,7 @@ FROM python:3.12-slim
|
|||||||
|
|
||||||
RUN apt-get update \
|
RUN apt-get update \
|
||||||
&& apt-get install -y --no-install-recommends \
|
&& 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/*
|
&& rm -rf /var/lib/apt/lists/*
|
||||||
|
|
||||||
WORKDIR /app
|
WORKDIR /app
|
||||||
|
|||||||
@@ -43,12 +43,15 @@ Die nachgelagerten Testprozesse erhalten separat konfigurierte `TEST_DB_*`-Zugan
|
|||||||
Dieser Benutzer darf ausschliesslich die isolierten Testschemas bearbeiten. Fuer Laravel
|
Dieser Benutzer darf ausschliesslich die isolierten Testschemas bearbeiten. Fuer Laravel
|
||||||
werden daraus `DB_*`, `ERP_DB_*` und `COCKPIT_CONFIG_DATABASE` gesetzt; Aider selbst sieht
|
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.
|
diese Werte nie. Produktive ERP- oder Kundenzugangsdaten duerfen hier nicht verwendet werden.
|
||||||
Vor jedem Testlauf initialisiert `php artisan migrate:fresh --force` das isolierte Laravel-
|
Vor jedem Testlauf erstellt `npm run build` die Vite-Assets und initialisiert
|
||||||
Schema neu. Deshalb muessen alle `TEST_DB_*`-Schemas dediziert und vollstaendig entbehrlich sein.
|
`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
|
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
|
des Hosts wird nicht eingebunden. Abgebrochene Claims werden nach standardmaessig 90
|
||||||
Minuten wieder freigegeben; `STALE_CLAIM_MINUTES` passt dieses Intervall an.
|
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
|
## Schnellstart
|
||||||
|
|
||||||
|
|||||||
@@ -21,8 +21,9 @@ services:
|
|||||||
AGENT_MODEL: ${AGENT_MODEL:-mistral/devstral-latest}
|
AGENT_MODEL: ${AGENT_MODEL:-mistral/devstral-latest}
|
||||||
MISTRAL_API_KEY: ${MISTRAL_API_KEY:-}
|
MISTRAL_API_KEY: ${MISTRAL_API_KEY:-}
|
||||||
POLL_SECONDS: ${POLL_SECONDS:-60}
|
POLL_SECONDS: ${POLL_SECONDS:-60}
|
||||||
|
MAX_AGENT_ATTEMPTS: ${MAX_AGENT_ATTEMPTS:-2}
|
||||||
STALE_CLAIM_MINUTES: ${STALE_CLAIM_MINUTES:-90}
|
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_HOST: ${TEST_DB_HOST:-host.docker.internal}
|
||||||
TEST_DB_PORT: ${TEST_DB_PORT:-33060}
|
TEST_DB_PORT: ${TEST_DB_PORT:-33060}
|
||||||
TEST_DB_NAME: ${TEST_DB_NAME:-laravel_ai_chat_agent_test}
|
TEST_DB_NAME: ${TEST_DB_NAME:-laravel_ai_chat_agent_test}
|
||||||
|
|||||||
@@ -43,6 +43,7 @@ class Settings:
|
|||||||
poll_seconds: int
|
poll_seconds: int
|
||||||
max_changed_files: int
|
max_changed_files: int
|
||||||
max_changed_lines: int
|
max_changed_lines: int
|
||||||
|
max_agent_attempts: int
|
||||||
stale_claim_minutes: int
|
stale_claim_minutes: int
|
||||||
dry_run: bool
|
dry_run: bool
|
||||||
|
|
||||||
@@ -75,7 +76,7 @@ class Settings:
|
|||||||
test_commands=_commands(
|
test_commands=_commands(
|
||||||
os.getenv(
|
os.getenv(
|
||||||
"TEST_COMMANDS",
|
"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"),
|
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")),
|
poll_seconds=int(os.getenv("POLL_SECONDS", "60")),
|
||||||
max_changed_files=int(os.getenv("MAX_CHANGED_FILES", "12")),
|
max_changed_files=int(os.getenv("MAX_CHANGED_FILES", "12")),
|
||||||
max_changed_lines=int(os.getenv("MAX_CHANGED_LINES", "800")),
|
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")),
|
stale_claim_minutes=int(os.getenv("STALE_CLAIM_MINUTES", "90")),
|
||||||
dry_run=mode == "dry-run",
|
dry_run=mode == "dry-run",
|
||||||
)
|
)
|
||||||
@@ -103,3 +105,5 @@ class Settings:
|
|||||||
raise ValueError("FAILURE_DB_PASSWORD is required")
|
raise ValueError("FAILURE_DB_PASSWORD is required")
|
||||||
if not self.test_database_password:
|
if not self.test_database_password:
|
||||||
raise ValueError("TEST_DB_PASSWORD is required")
|
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")
|
||||||
|
|||||||
@@ -71,3 +71,17 @@ Generated SQL, if relevant:
|
|||||||
|
|
||||||
Run focused tests while working. Leave the repository with the fix and regression test applied.
|
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.
|
||||||
|
"""
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ from .git import GitWorkspace
|
|||||||
from .gitea import GiteaClient
|
from .gitea import GiteaClient
|
||||||
from .models import FailureReport
|
from .models import FailureReport
|
||||||
from .policy import evaluate_changes
|
from .policy import evaluate_changes
|
||||||
from .prompt import build_task
|
from .prompt import build_retry_task, build_task
|
||||||
|
|
||||||
LOG = logging.getLogger(__name__)
|
LOG = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -43,15 +43,29 @@ class AutomationWorker:
|
|||||||
try:
|
try:
|
||||||
workspace.prepare()
|
workspace.prepare()
|
||||||
task = build_task(report)
|
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()
|
summary = workspace.summary()
|
||||||
metadata["changes"] = asdict(summary)
|
metadata["changes"] = asdict(summary)
|
||||||
policy = evaluate_changes(summary, self.settings, workspace.path)
|
policy = evaluate_changes(summary, self.settings, workspace.path)
|
||||||
if not policy.allowed:
|
if not policy.allowed:
|
||||||
raise RuntimeError("Policy rejected changes: " + "; ".join(policy.errors))
|
raise RuntimeError(
|
||||||
|
"Policy rejected changes: " + "; ".join(policy.errors)
|
||||||
|
)
|
||||||
|
try:
|
||||||
metadata["tests"] = self._run_tests(workspace)
|
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()
|
summary = workspace.summary()
|
||||||
metadata["changes"] = asdict(summary)
|
metadata["changes"] = asdict(summary)
|
||||||
policy = evaluate_changes(summary, self.settings, workspace.path)
|
policy = evaluate_changes(summary, self.settings, workspace.path)
|
||||||
@@ -101,6 +115,9 @@ class AutomationWorker:
|
|||||||
results = []
|
results = []
|
||||||
test_environment = {
|
test_environment = {
|
||||||
"AGENT_TEST_DB_ENABLED": "true",
|
"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_HOST": self.settings.test_database_host,
|
||||||
"TEST_DB_PORT": str(self.settings.test_database_port),
|
"TEST_DB_PORT": str(self.settings.test_database_port),
|
||||||
"TEST_DB_NAME": self.settings.test_database_name,
|
"TEST_DB_NAME": self.settings.test_database_name,
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ def settings(**overrides) -> Settings:
|
|||||||
"poll_seconds": 60,
|
"poll_seconds": 60,
|
||||||
"max_changed_files": 12,
|
"max_changed_files": 12,
|
||||||
"max_changed_lines": 800,
|
"max_changed_lines": 800,
|
||||||
|
"max_agent_attempts": 2,
|
||||||
"stale_claim_minutes": 90,
|
"stale_claim_minutes": 90,
|
||||||
"dry_run": True,
|
"dry_run": True,
|
||||||
}
|
}
|
||||||
@@ -61,5 +62,12 @@ def test_default_commands_initialize_disposable_laravel_schema(monkeypatch) -> N
|
|||||||
|
|
||||||
config = Settings.from_env()
|
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 "php artisan migrate:fresh --force" in config.test_commands
|
||||||
assert config.test_commands[-1] == "php artisan test"
|
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()
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
from lai_automation.models import FailureReport
|
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:
|
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 "eyJhbGci" not in result
|
||||||
assert "database-password" not in result
|
assert "database-password" not in result
|
||||||
assert "private-material" 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
|
||||||
|
|||||||
@@ -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 test_config import settings
|
||||||
|
|
||||||
|
from lai_automation.git import ChangeSummary
|
||||||
from lai_automation.models import FailureReport
|
from lai_automation.models import FailureReport
|
||||||
from lai_automation.worker import AutomationWorker
|
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
|
assert results[0]["returncode"] == 0
|
||||||
environment = workspace.run.call_args.kwargs["extra_environment"]
|
environment = workspace.run.call_args.kwargs["extra_environment"]
|
||||||
assert environment["AGENT_TEST_DB_ENABLED"] == "true"
|
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_USER"] == "test_runner"
|
||||||
assert environment["TEST_DB_PASSWORD"] == "test-secret"
|
assert environment["TEST_DB_PASSWORD"] == "test-secret"
|
||||||
assert environment["DB_DATABASE"] == "lai_agent_test"
|
assert environment["DB_DATABASE"] == "lai_agent_test"
|
||||||
assert environment["ERP_DATABASE"] == "customer_agent_test"
|
assert environment["ERP_DATABASE"] == "customer_agent_test"
|
||||||
assert environment["COCKPIT_CONFIG_DATABASE"] == "cockpit_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()
|
||||||
|
|||||||
Reference in New Issue
Block a user