This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
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("<?php\nreturn [];\n")
|
||||
subprocess.run(["git", "add", "."], cwd=workspace.path, check=True)
|
||||
subprocess.run(["git", "commit", "-m", "base"], cwd=workspace.path, check=True, capture_output=True)
|
||||
tracked.write_text("<?php\ninvalid\n")
|
||||
untracked = workspace.path / "extra.php"
|
||||
untracked.write_text("<?php\n")
|
||||
|
||||
workspace.discard_changes()
|
||||
|
||||
assert tracked.read_text() == "<?php\nreturn [];\n"
|
||||
assert not untracked.exists()
|
||||
@@ -176,3 +176,27 @@ def test_unresolved_placeholder_task_guides_real_answer_path() -> 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
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user