diff --git a/src/lai_automation/git.py b/src/lai_automation/git.py index 4f68059..32237aa 100644 --- a/src/lai_automation/git.py +++ b/src/lai_automation/git.py @@ -5,7 +5,7 @@ import pwd import shlex import shutil import subprocess -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from dataclasses import dataclass from pathlib import Path from urllib.parse import urlparse @@ -48,6 +48,20 @@ class GitWorkspace: check: bool = True, username: str | None = None, extra_environment: Mapping[str, str] | None = None, + ) -> subprocess.CompletedProcess[str]: + return self.run_args( + shlex.split(command), + check=check, + username=username, + extra_environment=extra_environment, + ) + + def run_args( + self, + command: Sequence[str], + check: bool = True, + username: str | None = None, + extra_environment: Mapping[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: environment = dict(os.environ) drop_privileges = None @@ -66,7 +80,7 @@ class GitWorkspace: environment.update(extra_environment or {}) return subprocess.run( - shlex.split(command), + command, cwd=self.path, env=environment, text=True, @@ -76,6 +90,10 @@ class GitWorkspace: preexec_fn=drop_privileges, ) + def discard_changes(self) -> None: + self._git("reset", "--hard", "HEAD") + self._git("clean", "-fd") + def summary(self) -> ChangeSummary: self._git("add", "-N", "--all") output = self._git("diff", "--numstat").stdout diff --git a/src/lai_automation/prompt.py b/src/lai_automation/prompt.py index ed87f2e..4d74891 100644 --- a/src/lai_automation/prompt.py +++ b/src/lai_automation/prompt.py @@ -25,6 +25,9 @@ Repository ownership map: - config/cockpit_semantics.php owns device and customer-database semantics. - app/Services/AIService.php owns provider calls, pseudonymization boundaries, and response parsing. - app/Services/TenantDatabaseService.php owns ERP/config/customer source selection and tenant metadata. +- Cross-database workflows are orchestrated in ChatController/TenantDatabaseService; one SQL query + cannot join tables exposed through different Laravel connections unless the tracked architecture + explicitly supports qualified access. - app/Services/SQLValidatorService.php owns read-only SQL safety; do not weaken it to accept a fix. - app/Http/Controllers/ChatController.php orchestrates routing, execution, answers, and diagnostics. - app/Services/FailureTrackingService.php and FailureAutomationService.php own failure handoff. @@ -44,6 +47,8 @@ Repair workflow: 8. Inspect the final diff for malformed edits, unrelated files, hardcoded report values, and tests that pass without exercising the changed behavior. Before retrying an edit, reread the current file and never leave conflict markers or partial methods behind. +9. PHP config files are executable code. Preserve brackets, commas, quotes, and array nesting; run + `php -l` on every changed PHP file before broader tests. """ @@ -91,6 +96,11 @@ For an unknown-column failure, compare every alias.column in the observed SQL wi schema and identify the owning table before editing. Teach the exact ownership, join, grouping, and real column names in semantics config, then add a reference SQL example for the reported question shape. Never replace one missing column with an unverified synonym. + +When expected behavior names tables from multiple databases or connections, first inspect source +routing and existing staged-query workflows. Do not put an impossible cross-connection join only +in ERP semantics. Implement deterministic orchestration that resolves identifiers in one source +and passes only the required identifier into the next source, preserving tenant isolation. """ placeholder_guidance = '' if '__LAI_' in response: diff --git a/src/lai_automation/worker.py b/src/lai_automation/worker.py index 98a1d66..d9cc556 100644 --- a/src/lai_automation/worker.py +++ b/src/lai_automation/worker.py @@ -11,7 +11,7 @@ from dataclasses import asdict from .agent import AiderAgent from .config import Settings from .database import FailureRepository -from .git import GitWorkspace +from .git import ChangeSummary, GitWorkspace from .gitea import GiteaClient from .models import FailureReport from .policy import evaluate_changes @@ -98,6 +98,25 @@ class AutomationWorker: raise RuntimeError( "Policy rejected changes: " + "; ".join(policy.errors) ) + try: + self._lint_changed_php(workspace, summary, report.id, attempt) + except RuntimeError as exception: + if attempt >= self.settings.max_agent_attempts: + raise + LOG.warning( + "PHP syntax failed for report %s on attempt %s; discarding malformed attempt", + report.id, + attempt, + ) + workspace.discard_changes() + task = build_retry_task( + report, + RuntimeError( + f"{exception}\nThe malformed attempt was discarded. Start again from the clean " + "repository, make smaller edits, and run php -l on each changed PHP file." + ), + ) + continue try: metadata["tests"] = self._run_tests(workspace, report.id, attempt) LOG.info("Report %s attempt %s validation passed", report.id, attempt) @@ -150,6 +169,26 @@ class AutomationWorker: except OSError: LOG.exception("Could not clean workspace for report %s", report.id) + def _lint_changed_php( + self, + workspace: GitWorkspace, + summary: ChangeSummary, + report_id: int, + attempt: int, + ) -> None: + for filename in summary.files: + if not filename.endswith(".php"): + continue + result = workspace.run_args( + ["php", "-l", filename], + check=False, + username="coder", + ) + if result.returncode != 0: + output = (result.stdout + "\n" + result.stderr)[-4_000:] + raise RuntimeError(f"PHP syntax check failed for {filename}:\n{output}") + LOG.info("Report %s attempt %s PHP syntax preflight passed", report_id, attempt) + def run_forever(self) -> None: LOG.info( "Worker started mode=%s model=%s poll_seconds=%s max_attempts=%s tests=%s direct_main=%s", diff --git a/tests/test_git.py b/tests/test_git.py index f8dbe32..8f567f8 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -25,4 +25,32 @@ def test_summary_includes_untracked_files(tmp_path) -> None: summary = workspace.summary() assert summary.files == ("new.php",) - assert summary.added_lines == 1 \ No newline at end of file + assert summary.added_lines == 1 + + +def test_discard_changes_restores_clean_disposable_workspace(tmp_path) -> None: + workspace = GitWorkspace(settings(workspace_root=tmp_path), 3) + workspace.path.mkdir() + subprocess.run(["git", "init"], cwd=workspace.path, check=True, capture_output=True) + subprocess.run( + ["git", "config", "user.email", "test@example.invalid"], + cwd=workspace.path, + check=True, + ) + subprocess.run( + ["git", "config", "user.name", "Test"], + cwd=workspace.path, + check=True, + ) + tracked = workspace.path / "tracked.php" + tracked.write_text(" None: assert "exact token map" in task assert "HTTP-backed AIService test" in task assert "local database-result fallback" in task + + +def test_cross_database_report_guides_staged_source_routing() -> None: + report = FailureReport( + id=56, + chat_message_id=1, + source="user_feedback", + category="incorrect_answer", + user_feedback="Wrong hours", + feedback_type="wrong_result", + expected_behavior="Join cockpit_config.time_registration to customer manager and ERP positions", + prompt="Hours in April", + response="0 hours", + sql_query="SELECT SUM(hours) FROM LX_AUFGABE", + error_message=None, + context={"database_type": "erp"}, + ) + + task = build_task(report) + + assert "tables from multiple databases" in task + assert "staged-query workflows" in task + assert "Do not put an impossible cross-connection join" in task + assert "preserving tenant isolation" in task diff --git a/tests/test_worker.py b/tests/test_worker.py index 182fa15..a357687 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -105,6 +105,52 @@ def test_long_stage_emits_heartbeat(caplog) -> None: assert "Report 7 agent attempt 1 finished" in caplog.text +def test_php_syntax_failure_discards_attempt_before_full_validation() -> None: + report = FailureReport( + id=56, + chat_message_id=1, + source="user_feedback", + category="incorrect_answer", + user_feedback="Wrong hours", + feedback_type="wrong_result", + expected_behavior="Use time registration", + prompt="Hours in April", + response="0 hours", + sql_query="SELECT 0", + error_message=None, + 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 = ["broken PHP", "fixed PHP"] + worker.gitea = Mock() + worker._run_tests = Mock(return_value=[]) + workspace = Mock() + workspace.path = Path("/tmp/nonexistent-workspace") + workspace.branch = "ai/failure-56" + workspace.summary.return_value = ChangeSummary( + ("config/erp_semantics.php", "tests/Unit/FixTest.php"), 20, 2 + ) + workspace.run_args.side_effect = [ + Mock(returncode=255, stdout="Parse error", stderr="unexpected token"), + Mock(returncode=0, stdout="No syntax errors", stderr=""), + Mock(returncode=0, stdout="No syntax errors", stderr=""), + ] + 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 "unexpected token" in worker.agent.run.call_args_list[1].args[1] + assert "clean repository" in worker.agent.run.call_args_list[1].args[1] + workspace.discard_changes.assert_called_once() + worker._run_tests.assert_called_once() + + def test_failed_validation_gets_one_correction_before_pr() -> None: report = FailureReport( id=13, @@ -134,6 +180,7 @@ def test_failed_validation_gets_one_correction_before_pr() -> None: workspace.summary.return_value = ChangeSummary( ("app/Services/Fix.php", "tests/Feature/FixTest.php"), 10, 1 ) + workspace.run_args.return_value = Mock(returncode=0, stdout="No syntax errors", stderr="") workspace.commit.return_value = "commit-id" with patch("lai_automation.worker.GitWorkspace", return_value=workspace): @@ -177,6 +224,7 @@ def test_no_change_response_gets_one_correction() -> None: ChangeSummary(("app/Services/Fix.php", "tests/Feature/FixTest.php"), 10, 1), ChangeSummary(("app/Services/Fix.php", "tests/Feature/FixTest.php"), 10, 1), ] + workspace.run_args.return_value = Mock(returncode=0, stdout="No syntax errors", stderr="") workspace.commit.return_value = "commit-id" with patch("lai_automation.worker.GitWorkspace", return_value=workspace): @@ -219,6 +267,7 @@ def test_test_only_change_gets_correction_before_validation() -> None: ChangeSummary(("app/Services/Fix.php", "tests/Unit/FixTest.php"), 15, 1), ChangeSummary(("app/Services/Fix.php", "tests/Unit/FixTest.php"), 15, 1), ] + workspace.run_args.return_value = Mock(returncode=0, stdout="No syntax errors", stderr="") workspace.commit.return_value = "commit-id" with patch("lai_automation.worker.GitWorkspace", return_value=workspace):