mirror of
https://github.com/usestrix/strix.git
synced 2026-10-01 02:03:55 +00:00
Let repair and review agents own the fix workflow
This commit is contained in:
parent
0faa7b7da1
commit
43391ebefa
3 changed files with 278 additions and 498 deletions
|
|
@ -10,6 +10,8 @@ from typing import TYPE_CHECKING, Literal, cast
|
|||
|
||||
from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
|
||||
|
||||
from strix.config.settings import DEFAULT_MAX_TURNS
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Mapping
|
||||
|
|
@ -133,7 +135,7 @@ class CommandSpec(ContractModel):
|
|||
required: bool = True
|
||||
timeout_seconds: int = Field(default=300, ge=1, le=3600)
|
||||
cwd: str = "."
|
||||
purpose: Literal["quality", "regression", "unit"] = "quality"
|
||||
purpose: Literal["quality", "regression", "unit", "security"] = "quality"
|
||||
|
||||
_relative_cwd = field_validator("cwd")(_validate_relative_path)
|
||||
|
||||
|
|
@ -151,7 +153,7 @@ class ReproductionSpec(ContractModel):
|
|||
|
||||
|
||||
class RepositoryTestPlan(ContractModel):
|
||||
"""The repair agent's native tests, rerun by the controller before review."""
|
||||
"""Historical native-test handoff; agent-driven runs record commands directly."""
|
||||
|
||||
regression_test: CommandSpec
|
||||
regression_files: list[str] = Field(min_length=1)
|
||||
|
|
@ -218,8 +220,11 @@ class FixPreparationRequestV1(ContractModel):
|
|||
repository_id: str | None = None
|
||||
candidate: FixCandidateV1
|
||||
checks: list[CommandSpec] = []
|
||||
max_repair_attempts: int = Field(default=2, ge=1, le=2)
|
||||
timeout_seconds: int = Field(default=1800, ge=30, le=14400)
|
||||
# Accepted for old callers; the agent loop is bounded by turns/time instead.
|
||||
max_repair_attempts: int | None = Field(default=None, ge=1)
|
||||
max_agent_turns: int = Field(default=DEFAULT_MAX_TURNS, ge=1, le=10000)
|
||||
timeout_seconds: int = Field(default=7200, ge=30, le=14400)
|
||||
max_budget_usd: float | None = Field(default=None, gt=0, allow_inf_nan=False)
|
||||
network_allowed: bool = False
|
||||
credentials_allowed: list[str] = []
|
||||
|
||||
|
|
@ -244,7 +249,7 @@ class CheckResult(ContractModel):
|
|||
baseline_workspace_root: str | None = None
|
||||
source_digest: str | None = Field(default=None, pattern=r"^[0-9a-f]{64}$")
|
||||
environment_id: str | None = None
|
||||
purpose: Literal["quality", "regression", "unit"] = "quality"
|
||||
purpose: Literal["quality", "regression", "unit", "security"] = "quality"
|
||||
tests_passed: int | None = Field(default=None, ge=0)
|
||||
|
||||
|
||||
|
|
@ -316,6 +321,8 @@ class VerifierResult(ContractModel):
|
|||
unit_test_coverage_valid: bool = False
|
||||
# None preserves historical reviews; new reviewers classify every concern.
|
||||
concerns: list[ReviewConcern] | None = None
|
||||
source_digest: str | None = Field(default=None, pattern=r"^[0-9a-f]{64}$")
|
||||
turns_used: int = Field(default=0, ge=0)
|
||||
|
||||
|
||||
class RepairOutcome(ContractModel):
|
||||
|
|
@ -328,6 +335,7 @@ class RepairOutcome(ContractModel):
|
|||
checks: list[CommandSpec] = []
|
||||
command_results: list[CheckResult] = []
|
||||
test_plan: RepositoryTestPlan | None = None
|
||||
source_digest: str | None = Field(default=None, pattern=r"^[0-9a-f]{64}$")
|
||||
|
||||
|
||||
class FixPreparationAttempt(ContractModel):
|
||||
|
|
@ -352,7 +360,8 @@ class FileManifestEntry(ContractModel):
|
|||
class FixPreparationResultV1(ContractModel):
|
||||
version: Literal["1"] = "1"
|
||||
# Absent on historical records; keep their stronger, paired-proof interpretation.
|
||||
validation_mode: Literal["paired", "native_tests"] = "paired"
|
||||
validation_mode: Literal["paired", "native_tests", "agent_review"] = "paired"
|
||||
prepared_source_digest: str | None = Field(default=None, pattern=r"^[0-9a-f]{64}$")
|
||||
state: PreparationState
|
||||
stop_reason: str
|
||||
source_identity: SourceIdentity | None
|
||||
|
|
|
|||
|
|
@ -32,8 +32,7 @@ from strix.fix.contracts import (
|
|||
VerificationDecision,
|
||||
VerifierResult,
|
||||
)
|
||||
from strix.fix.evidence import command_status, record_test_execution
|
||||
from strix.fix.locations import AnchorStatus, anchor_location
|
||||
from strix.fix.evidence import command_status
|
||||
|
||||
|
||||
class PreparationCancelledError(RuntimeError):
|
||||
|
|
@ -47,14 +46,9 @@ CancellationCheck = Callable[[], bool]
|
|||
|
||||
@dataclass(slots=True)
|
||||
class PreparationPolicy:
|
||||
max_repair_attempts: int = 2
|
||||
timeout_seconds: int = 1800
|
||||
timeout_seconds: int = 7200
|
||||
max_output_chars: int = 20000
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
if not 1 <= self.max_repair_attempts <= 2:
|
||||
raise ValueError("max_repair_attempts must be between 1 and 2")
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class PreparationContext:
|
||||
|
|
@ -74,7 +68,7 @@ IndependentVerifier = Callable[
|
|||
Awaitable[VerifierResult],
|
||||
]
|
||||
SourceVerifier = Callable[[PreparationContext], Awaitable[bool]]
|
||||
CheckPlanner = Callable[[PreparationContext, list[FileManifestEntry]], Awaitable[list[CommandSpec]]]
|
||||
EvidenceReader = Callable[[], Awaitable[list[CheckResult]]]
|
||||
|
||||
|
||||
_COMMAND_ENV_ALLOWLIST = frozenset(
|
||||
|
|
@ -378,7 +372,10 @@ def _result(
|
|||
) -> FixPreparationResultV1:
|
||||
return FixPreparationResultV1(
|
||||
state=state,
|
||||
validation_mode="native_tests",
|
||||
validation_mode="agent_review",
|
||||
prepared_source_digest=(
|
||||
context.feedback[-1].repair.source_digest if context.feedback else None
|
||||
),
|
||||
test_plan=context.feedback[-1].repair.test_plan if context.feedback else None,
|
||||
stop_reason=reason,
|
||||
source_identity=context.candidate.source_identity,
|
||||
|
|
@ -408,128 +405,53 @@ def _repair_outcome(value: RepairOutcome | None) -> RepairOutcome:
|
|||
)
|
||||
|
||||
|
||||
def _required_checks_pass(checks: list[CheckResult]) -> bool:
|
||||
required = [result for result in checks if result.required]
|
||||
return bool(required) and all(
|
||||
result.status is CheckStatus.PASSED and result.exit_code == 0 for result in required
|
||||
)
|
||||
|
||||
|
||||
def _verification_passes(checks: list[CheckResult], verifier: VerifierResult) -> bool:
|
||||
return (
|
||||
_required_checks_pass(checks)
|
||||
and any(
|
||||
item.purpose == "regression" and item.status is CheckStatus.PASSED for item in checks
|
||||
)
|
||||
and any(
|
||||
item.purpose == "unit" and item.required and item.status is CheckStatus.PASSED
|
||||
def validation_gaps(
|
||||
checks: list[CheckResult], source_digest: str | None, requested: list[CommandSpec]
|
||||
) -> list[str]:
|
||||
"""Check execution facts only; agents judge coverage and the security fix."""
|
||||
gaps: list[str] = []
|
||||
if not source_digest:
|
||||
gaps.append("No source digest was recorded for the prepared patch.")
|
||||
for purpose in ("regression", "unit"):
|
||||
if not any(item.purpose == purpose and item.required for item in checks):
|
||||
gaps.append(f"Run and record the required {purpose} tests.") # noqa: PERF401
|
||||
for item in checks:
|
||||
if not item.required:
|
||||
continue
|
||||
if item.source_digest != source_digest or not item.environment_id:
|
||||
gaps.append(f"Rerun {item.name}: its results are not for the current patch.")
|
||||
elif item.status is not CheckStatus.PASSED or item.exit_code != 0 or item.tests_passed == 0:
|
||||
gaps.append(f"Resolve and rerun {item.name}: {item.status}.")
|
||||
if len({item.environment_id for item in checks if item.required}) > 1:
|
||||
gaps.append("Required checks refer to different execution environments.")
|
||||
for command in requested:
|
||||
if command.required and not any(
|
||||
item.required and item.argv == command.argv and item.cwd == command.cwd
|
||||
for item in checks
|
||||
)
|
||||
and len({item.environment_id for item in checks if item.environment_id}) <= 1
|
||||
and len({item.source_digest for item in checks if item.source_digest}) <= 1
|
||||
and verifier.decision is VerificationDecision.VERIFIED
|
||||
and verifier.security_invariant_closed
|
||||
and verifier.regression_test_valid
|
||||
and verifier.unit_test_coverage_valid
|
||||
and verifier.review_basis in {"execution", "code_review"}
|
||||
and not verifier.gaps
|
||||
and verifier.blocker is None
|
||||
)
|
||||
|
||||
|
||||
def _test_plan_gaps(repair: RepairOutcome, manifest: list[FileManifestEntry]) -> list[str]:
|
||||
plan = repair.test_plan
|
||||
if plan is None:
|
||||
return ["The repair must supply a regression test and the repository unit-test commands."]
|
||||
changed = {entry.path for entry in manifest if entry.operation != "delete"}
|
||||
missing = [path for path in plan.regression_files if path not in changed]
|
||||
gaps = [f"Regression test must be added or updated in the patch: {path}" for path in missing]
|
||||
if not plan.unit_tests:
|
||||
gaps.append("Supply the customer's existing unit-test suite for the affected code.")
|
||||
if plan.no_unit_tests_reason:
|
||||
gaps.append(plan.no_unit_tests_reason)
|
||||
):
|
||||
gaps.append(f"Run the requested check: {command.name}.") # noqa: PERF401
|
||||
return gaps
|
||||
|
||||
|
||||
def _resolve_review(verifier: VerifierResult) -> VerifierResult:
|
||||
"""Derive the outcome from actionable concerns, retaining legacy record support."""
|
||||
if verifier.concerns is None:
|
||||
return verifier
|
||||
repairs = [c.summary for c in verifier.concerns if c.kind == "repair_needed"]
|
||||
prerequisites = [c.summary for c in verifier.concerns if c.kind == "customer_prerequisite"]
|
||||
notes = [c.summary for c in verifier.concerns if c.kind == "optional_follow_up"]
|
||||
approved = (
|
||||
verifier.security_invariant_closed
|
||||
and verifier.regression_test_valid
|
||||
and verifier.unit_test_coverage_valid
|
||||
)
|
||||
decision = (
|
||||
VerificationDecision.REJECTED
|
||||
if repairs
|
||||
else VerificationDecision.INCONCLUSIVE
|
||||
if prerequisites or not approved
|
||||
else VerificationDecision.VERIFIED
|
||||
)
|
||||
return verifier.model_copy(
|
||||
update={
|
||||
"decision": decision,
|
||||
"repairable": bool(repairs),
|
||||
"gaps": [*repairs, *prerequisites],
|
||||
"notes": list(dict.fromkeys([*verifier.notes, *notes])),
|
||||
"blocker": PreparationBlocker(
|
||||
kind=BlockerKind.EXTERNAL_CONFIGURATION,
|
||||
summary="A required customer prerequisite is missing.",
|
||||
user_action="\n".join(prerequisites),
|
||||
details=prerequisites,
|
||||
)
|
||||
if prerequisites
|
||||
else None,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _test_commands(repair: RepairOutcome) -> list[CommandSpec]:
|
||||
plan = repair.test_plan
|
||||
if plan is None:
|
||||
return []
|
||||
return [
|
||||
plan.regression_test.model_copy(update={"purpose": "regression", "required": True}),
|
||||
*(
|
||||
item.model_copy(update={"purpose": "unit", "required": True})
|
||||
for item in plan.unit_tests
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
async def prepare_fix( # noqa: PLR0915
|
||||
async def prepare_fix( # noqa: PLR0915 - thin orchestration and cleanup
|
||||
request: FixPreparationRequestV1,
|
||||
workspace: Path,
|
||||
*,
|
||||
repair: RepairAgent,
|
||||
verify: IndependentVerifier,
|
||||
command_runner: CommandRunner = run_command,
|
||||
manifest_builder: ManifestBuilder = build_git_manifest,
|
||||
source_verifier: SourceVerifier = _verify_source,
|
||||
check_planner: CheckPlanner | None = None,
|
||||
evidence_reader: EvidenceReader | None = None,
|
||||
cancelled: CancellationCheck = lambda: False,
|
||||
policy: PreparationPolicy | None = None,
|
||||
) -> FixPreparationResultV1:
|
||||
"""Run repair/review conversations; agents own setup, tests and corrections."""
|
||||
started = time.monotonic()
|
||||
resolved_policy = policy or PreparationPolicy(
|
||||
max_repair_attempts=request.max_repair_attempts,
|
||||
timeout_seconds=request.timeout_seconds,
|
||||
)
|
||||
resolved_policy = policy or PreparationPolicy(timeout_seconds=request.timeout_seconds)
|
||||
context = PreparationContext(request=request, workspace=workspace, candidate=request.candidate)
|
||||
runner = command_runner
|
||||
if runner is run_command:
|
||||
runner = functools.partial(
|
||||
run_command,
|
||||
credentials_allowed=request.credentials_allowed,
|
||||
network_allowed=request.network_allowed,
|
||||
)
|
||||
checks: list[CheckResult] = []
|
||||
verifier: VerifierResult | None = None
|
||||
reproduction: CheckResult | None = None
|
||||
repair_turns = review_turns = 0
|
||||
|
||||
async def finish(
|
||||
state: PreparationState,
|
||||
|
|
@ -538,201 +460,110 @@ async def prepare_fix( # noqa: PLR0915
|
|||
blocker: PreparationBlocker | None = None,
|
||||
gaps: list[str] | None = None,
|
||||
) -> FixPreparationResultV1:
|
||||
nonlocal checks
|
||||
if evidence_reader:
|
||||
checks = await evidence_reader()
|
||||
manifest, summary, artifact = await manifest_builder(workspace)
|
||||
retained_verifier = verifier or (
|
||||
context.feedback[-1].verifier if context.feedback else None
|
||||
)
|
||||
return _result(
|
||||
context,
|
||||
state=state,
|
||||
reason=reason,
|
||||
started=started,
|
||||
checks=checks,
|
||||
verifier=retained_verifier,
|
||||
reproduction=reproduction,
|
||||
verifier=verifier,
|
||||
manifest=manifest,
|
||||
diff_summary=summary,
|
||||
artifact_ref=artifact,
|
||||
attempt_history=context.feedback,
|
||||
blocker=blocker,
|
||||
gaps=gaps,
|
||||
reproduction=next((c for c in checks if c.purpose == "regression"), None),
|
||||
)
|
||||
|
||||
async def execute() -> FixPreparationResultV1: # noqa: PLR0911, PLR0912, PLR0915
|
||||
nonlocal checks, verifier, reproduction
|
||||
blocker: PreparationBlocker | None
|
||||
async def execute() -> FixPreparationResultV1: # noqa: PLR0912
|
||||
nonlocal checks, verifier, repair_turns, review_turns
|
||||
if cancelled():
|
||||
raise PreparationCancelledError
|
||||
if not await source_verifier(context):
|
||||
blocker = PreparationBlocker(
|
||||
kind=BlockerKind.SOURCE,
|
||||
summary="The repository no longer matches the finding source.",
|
||||
user_action="Refresh the finding against the current repository revision.",
|
||||
return await finish(
|
||||
PreparationState.STALE,
|
||||
"The repository no longer matches the finding source.",
|
||||
blocker=PreparationBlocker(
|
||||
kind=BlockerKind.SOURCE,
|
||||
summary="The repository no longer matches the finding source.",
|
||||
user_action="Refresh the finding against the current repository revision.",
|
||||
),
|
||||
)
|
||||
return _result(
|
||||
context,
|
||||
state=PreparationState.STALE,
|
||||
reason=blocker.summary,
|
||||
blocker=blocker,
|
||||
started=started,
|
||||
)
|
||||
anchors = [
|
||||
anchor_location(workspace, location, exact_source=True)
|
||||
for location in context.candidate.finding_locations
|
||||
]
|
||||
unresolved = [item for item in anchors if item.status is not AnchorStatus.UNIQUE]
|
||||
if unresolved:
|
||||
blocker = PreparationBlocker(
|
||||
kind=BlockerKind.SOURCE,
|
||||
summary="The reported finding locations could not be resolved uniquely.",
|
||||
user_action="Refresh the finding or identify the affected source location.",
|
||||
details=[f"{item.location.file}: {item.status}" for item in unresolved],
|
||||
)
|
||||
return await finish(PreparationState.BLOCKED, blocker.summary, blocker=blocker)
|
||||
context.candidate = context.candidate.model_copy(
|
||||
update={"finding_locations": [item.location for item in anchors]}
|
||||
)
|
||||
for attempt in range(1, resolved_policy.max_repair_attempts + 1):
|
||||
context.attempt = attempt
|
||||
# Location/snippet interpretation belongs to repair. Exact source identity is checked above.
|
||||
while repair_turns < request.max_agent_turns and review_turns < request.max_agent_turns:
|
||||
if cancelled():
|
||||
raise PreparationCancelledError
|
||||
context.attempt += 1
|
||||
verifier = None
|
||||
reproduction = None
|
||||
record = FixPreparationAttempt(
|
||||
attempt=attempt,
|
||||
attempt=context.attempt,
|
||||
repair=RepairOutcome(status=RepairStatus.INCOMPLETE, summary="Repair started."),
|
||||
workspace_digest=await _workspace_digest(workspace),
|
||||
)
|
||||
context.feedback.append(record)
|
||||
record.repair = _repair_outcome(await repair(context, checks))
|
||||
checks = record.checks
|
||||
repair_turns += max(1, record.repair.turns_used)
|
||||
checks = await evidence_reader() if evidence_reader else record.repair.command_results
|
||||
record.checks = list(checks)
|
||||
record.workspace_digest = await _workspace_digest(workspace)
|
||||
manifest, _, _ = await build_git_manifest(workspace)
|
||||
can_retry = (
|
||||
attempt < resolved_policy.max_repair_attempts
|
||||
and record.repair.status is not RepairStatus.BLOCKED
|
||||
)
|
||||
if not manifest:
|
||||
if record.repair.blocker:
|
||||
return await finish(
|
||||
PreparationState.BLOCKED,
|
||||
record.repair.summary,
|
||||
blocker=record.repair.blocker,
|
||||
gaps=record.repair.gaps,
|
||||
)
|
||||
record.repair.gaps.append(
|
||||
"No changed files were found. Use the file tools to implement the fix and "
|
||||
"regression test, then submit their validation commands."
|
||||
)
|
||||
if can_retry:
|
||||
continue
|
||||
return await finish(
|
||||
PreparationState.FAILED,
|
||||
"The repair did not change repository source.",
|
||||
gaps=record.repair.gaps,
|
||||
)
|
||||
await manifest_builder(workspace) # Save the patch before any test can fail.
|
||||
plan_gaps = _test_plan_gaps(record.repair, manifest)
|
||||
if plan_gaps:
|
||||
record.repair.gaps = list(dict.fromkeys([*record.repair.gaps, *plan_gaps]))
|
||||
if can_retry:
|
||||
continue
|
||||
return await finish(
|
||||
PreparationState.BLOCKED,
|
||||
"The patch is saved, but its required test handoff is incomplete.",
|
||||
blocker=record.repair.blocker,
|
||||
gaps=record.repair.gaps,
|
||||
)
|
||||
additional = await check_planner(context, manifest) if check_planner else request.checks
|
||||
planned = [*_test_commands(record.repair), *additional]
|
||||
unique: dict[tuple[str, tuple[str, ...], str], CommandSpec] = {}
|
||||
for command in planned:
|
||||
key = (command.purpose, tuple(command.argv), command.cwd)
|
||||
previous = unique.get(key)
|
||||
unique[key] = command.model_copy(
|
||||
update={"required": command.required or bool(previous and previous.required)}
|
||||
)
|
||||
checks = record.checks
|
||||
for command in unique.values():
|
||||
if cancelled():
|
||||
raise PreparationCancelledError
|
||||
checks.append(record_test_execution(await runner(workspace, command), command))
|
||||
reproduction = next((item for item in checks if item.purpose == "regression"), None)
|
||||
record.security_reproduction = reproduction
|
||||
failed = [
|
||||
item
|
||||
for item in checks
|
||||
if item.required and (item.status is not CheckStatus.PASSED or item.exit_code != 0)
|
||||
]
|
||||
if failed:
|
||||
if can_retry:
|
||||
continue
|
||||
return await finish(
|
||||
PreparationState.BLOCKED,
|
||||
"The patch is saved, but required tests or checks have not passed.",
|
||||
blocker=PreparationBlocker(
|
||||
kind=BlockerKind.VERIFICATION_RUNTIME,
|
||||
summary="Required tests or checks have not passed.",
|
||||
user_action="Review the recorded failures and retry after resolving them.",
|
||||
details=[f"{item.name}: {item.status}" for item in failed],
|
||||
),
|
||||
gaps=[f"{item.name}: {item.status}" for item in failed],
|
||||
)
|
||||
# Review the exact repair and its native test results. No second harness is required.
|
||||
verifier = _resolve_review(await verify(context, checks))
|
||||
record.verifier = verifier
|
||||
if record.workspace_digest != await _workspace_digest(workspace):
|
||||
return await finish(
|
||||
PreparationState.FAILED,
|
||||
"Review changed the prepared source; validation must be rerun.",
|
||||
)
|
||||
if verifier.decision is VerificationDecision.REJECTED:
|
||||
if verifier.repairable and can_retry:
|
||||
continue
|
||||
return await finish(
|
||||
PreparationState.FAILED,
|
||||
"The independent reviewer found a repair or regression-test defect.",
|
||||
gaps=verifier.gaps,
|
||||
)
|
||||
if verifier.blocker:
|
||||
return await finish(
|
||||
PreparationState.BLOCKED,
|
||||
verifier.blocker.summary,
|
||||
blocker=verifier.blocker,
|
||||
gaps=verifier.gaps,
|
||||
)
|
||||
if record.repair.status is RepairStatus.BLOCKED:
|
||||
manifest, _, _ = await manifest_builder(workspace)
|
||||
if record.repair.status is not RepairStatus.COMPLETE:
|
||||
return await finish(
|
||||
PreparationState.BLOCKED,
|
||||
record.repair.summary,
|
||||
blocker=record.repair.blocker,
|
||||
gaps=record.repair.gaps,
|
||||
)
|
||||
# Typed review explicitly resolves the repair agent's observations. Historical
|
||||
# free-text gaps remain blocking until a reviewer classifies them.
|
||||
repair_gaps = record.repair.gaps if verifier.concerns is None else []
|
||||
if _verification_passes(checks, verifier) and not repair_gaps:
|
||||
return await finish(
|
||||
PreparationState.READY,
|
||||
"Tests and required checks passed; independent review approved a draft PR.",
|
||||
if not manifest:
|
||||
record.repair.gaps.append(
|
||||
"No changed files were found. Implement the fix and regression test."
|
||||
)
|
||||
gaps = list(dict.fromkeys([*repair_gaps, *verifier.gaps]))
|
||||
if not gaps:
|
||||
gaps = ["Independent review could not confirm the repair and native-test coverage."]
|
||||
continue
|
||||
if cancelled():
|
||||
raise PreparationCancelledError
|
||||
# Review can investigate even incomplete validation and run the missing checks itself.
|
||||
verifier = await verify(context, checks)
|
||||
review_turns += max(1, verifier.turns_used)
|
||||
record.verifier = verifier
|
||||
if evidence_reader:
|
||||
checks = await evidence_reader()
|
||||
record.checks = list(checks)
|
||||
if verifier.blocker:
|
||||
return await finish(
|
||||
PreparationState.BLOCKED,
|
||||
verifier.summary,
|
||||
blocker=verifier.blocker,
|
||||
gaps=verifier.gaps,
|
||||
)
|
||||
if verifier.decision is VerificationDecision.REJECTED:
|
||||
record.repair.gaps.extend(verifier.gaps or [verifier.summary])
|
||||
continue
|
||||
if verifier.decision is not VerificationDecision.VERIFIED:
|
||||
return await finish(PreparationState.BLOCKED, verifier.summary, gaps=verifier.gaps)
|
||||
gaps = validation_gaps(checks, record.repair.source_digest, request.checks)
|
||||
if (
|
||||
record.workspace_digest != await _workspace_digest(workspace)
|
||||
or verifier.source_digest != record.repair.source_digest
|
||||
):
|
||||
gaps.append(
|
||||
"The deliverable changed during review. Inspect the diff, clean up temporary "
|
||||
"files, and rerun affected checks before requesting review again."
|
||||
)
|
||||
if gaps:
|
||||
record.repair.gaps.extend(gaps)
|
||||
continue
|
||||
return await finish(
|
||||
PreparationState.BLOCKED,
|
||||
"The patch and test results are saved, but independent review is incomplete.",
|
||||
blocker=verifier.blocker
|
||||
or PreparationBlocker(
|
||||
kind=BlockerKind.VERIFICATION_RUNTIME,
|
||||
summary="Independent review is incomplete.",
|
||||
user_action="Retry review of the saved changes and test evidence.",
|
||||
details=gaps,
|
||||
),
|
||||
gaps=gaps,
|
||||
PreparationState.READY,
|
||||
"Required tests passed and independent review approved the draft PR.",
|
||||
)
|
||||
return await finish(
|
||||
PreparationState.FAILED, "The repair exhausted its configured attempts."
|
||||
PreparationState.BLOCKED,
|
||||
"The agent turn budget was reached; partial work was retained.",
|
||||
)
|
||||
|
||||
try:
|
||||
|
|
@ -746,7 +577,6 @@ async def prepare_fix( # noqa: PLR0915
|
|||
logging.getLogger(__name__).exception(
|
||||
"Fix preparation failed during attempt %s", context.attempt
|
||||
)
|
||||
# Preserve partial work without exposing unredacted exception text.
|
||||
return await finish(
|
||||
PreparationState.FAILED,
|
||||
f"Fix preparation stopped after {type(error).__name__}; partial work was retained.",
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import hashlib
|
||||
import json
|
||||
import subprocess
|
||||
|
|
@ -23,9 +24,7 @@ from strix.fix.contracts import (
|
|||
RegressionTestResult,
|
||||
RepairOutcome,
|
||||
RepairStatus,
|
||||
RepositoryTestPlan,
|
||||
ReproductionSpec,
|
||||
ReviewConcern,
|
||||
SourceIdentity,
|
||||
SourceIdentityKind,
|
||||
VerificationDecision,
|
||||
|
|
@ -37,7 +36,9 @@ from strix.fix.evidence import command_status, passed_test_count, record_test_ex
|
|||
from strix.fix.locations import AnchorStatus, anchor_location
|
||||
from strix.fix.prepare import (
|
||||
PreparationContext,
|
||||
PreparationPolicy,
|
||||
_network_isolation_prefix,
|
||||
_workspace_digest,
|
||||
build_git_manifest,
|
||||
build_git_patch,
|
||||
prepare_fix,
|
||||
|
|
@ -160,6 +161,7 @@ def _request(candidate: FixCandidateV1, *, attempts: int = 2) -> FixPreparationR
|
|||
)
|
||||
],
|
||||
max_repair_attempts=attempts,
|
||||
max_agent_turns=8,
|
||||
network_allowed=True,
|
||||
)
|
||||
|
||||
|
|
@ -178,20 +180,34 @@ async def _noop_repair(
|
|||
"import unittest\nfrom app import result\nclass SecurityTest(unittest.TestCase):\n"
|
||||
" def test_safe(self): self.assertEqual(result(), 'safe')\n"
|
||||
)
|
||||
return RepairOutcome(
|
||||
test_plan=RepositoryTestPlan(
|
||||
regression_files=["test_app.py"],
|
||||
regression_test=CommandSpec(
|
||||
name="regression", argv=[sys.executable, "-m", "unittest", "test_app"]
|
||||
),
|
||||
unit_tests=[
|
||||
CommandSpec(
|
||||
name="existing suite", argv=[sys.executable, "-m", "unittest", "test_existing"]
|
||||
)
|
||||
],
|
||||
commands = [
|
||||
CommandSpec(
|
||||
name="regression",
|
||||
argv=[sys.executable, "-m", "unittest", "test_app"],
|
||||
purpose="regression",
|
||||
),
|
||||
CommandSpec(
|
||||
name="existing suite",
|
||||
argv=[sys.executable, "-m", "unittest", "test_existing"],
|
||||
purpose="unit",
|
||||
),
|
||||
*context.request.checks,
|
||||
]
|
||||
digest = await _workspace_digest(context.workspace)
|
||||
results = []
|
||||
for command in commands:
|
||||
result = record_test_execution(
|
||||
await run_command(context.workspace, command, network_allowed=True), command
|
||||
)
|
||||
results.append(
|
||||
result.model_copy(update={"source_digest": digest, "environment_id": "test"})
|
||||
)
|
||||
return RepairOutcome(
|
||||
status=RepairStatus.COMPLETE,
|
||||
summary="The repository fix is ready for independent evaluation.",
|
||||
summary="Fixed and tested.",
|
||||
command_results=results,
|
||||
source_digest=digest,
|
||||
turns_used=1,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -201,6 +217,7 @@ async def _verified(
|
|||
) -> VerifierResult:
|
||||
return VerifierResult(
|
||||
decision=VerificationDecision.VERIFIED,
|
||||
source_digest=_context.feedback[-1].repair.source_digest,
|
||||
summary="The invariant is closed.",
|
||||
security_invariant_closed=True,
|
||||
review_basis="code_review",
|
||||
|
|
@ -475,252 +492,176 @@ def test_regression_requires_consistent_execution_provenance() -> None:
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_native_flow_runs_regression_before_review_and_saves_test(tmp_path: Path) -> None:
|
||||
async def test_agent_tests_are_reused_without_controller_execution(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def review(context, checks):
|
||||
assert checks[0].purpose == "regression"
|
||||
assert checks[0].tests_passed == 1
|
||||
assert checks[0].exit_code == 0
|
||||
return await _verified(context, checks)
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=_noop_repair, verify=review
|
||||
)
|
||||
assert result.state is PreparationState.READY, result.model_dump_json()
|
||||
assert result.validation_mode == "native_tests"
|
||||
assert result.test_plan.regression_files == ["test_app.py"]
|
||||
assert set(result.changed_files) == {"app.py", "test_app.py"}
|
||||
assert result.verifier.review_basis == "code_review"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_failing_unit_suite_is_returned_to_repair_before_review(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
calls = []
|
||||
outcome = None
|
||||
|
||||
async def repair(context, checks):
|
||||
calls.append(context.attempt)
|
||||
nonlocal outcome
|
||||
outcome = await _noop_repair(context, checks)
|
||||
(workspace / "test_existing.py").write_text(
|
||||
"import unittest\nclass Existing(unittest.TestCase):\n"
|
||||
f" def test_existing(self): self.assertTrue({context.attempt > 1})\n"
|
||||
)
|
||||
outcome.test_plan.unit_tests = [
|
||||
CommandSpec(
|
||||
name="existing suite", argv=[sys.executable, "-m", "unittest", "test_existing"]
|
||||
)
|
||||
]
|
||||
outcome.test_plan.no_unit_tests_reason = None
|
||||
if context.attempt == 2:
|
||||
assert any(c.purpose == "unit" and c.status is CheckStatus.FAILED for c in checks)
|
||||
return outcome
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=repair, verify=_verified
|
||||
)
|
||||
assert result.state is PreparationState.READY
|
||||
assert calls == [1, 2]
|
||||
assert result.attempt_history[0].verifier is None
|
||||
assert result.checks[1].tests_passed == 1
|
||||
assert result.validation_mode == "agent_review"
|
||||
assert result.test_plan is None
|
||||
assert result.checks == outcome.command_results
|
||||
assert result.checks[0].tests_passed == 1
|
||||
assert result.prepared_source_digest == result.verifier.source_digest
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"output", ["Ran 0 tests in 0.000s\n\nOK", "3 skipped", "collected 0 items"]
|
||||
)
|
||||
async def test_empty_or_skipped_tests_never_approve(tmp_path: Path, output: str) -> None:
|
||||
async def test_review_can_request_more_than_two_repairs(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def runner(_root, command):
|
||||
return CheckResult(
|
||||
name=command.name,
|
||||
argv=command.argv,
|
||||
status=CheckStatus.PASSED,
|
||||
exit_code=0,
|
||||
output=output,
|
||||
duration_seconds=0,
|
||||
)
|
||||
async def review(context, checks):
|
||||
if context.attempt < 4:
|
||||
return VerifierResult(
|
||||
decision=VerificationDecision.REJECTED,
|
||||
summary="Inspect sibling path",
|
||||
gaps=["Inspect sibling path"],
|
||||
)
|
||||
assert context.feedback[-2].verifier.summary == "Inspect sibling path"
|
||||
return await _verified(context, checks)
|
||||
|
||||
async def review(*_args):
|
||||
pytest.fail("Review must not run without passing tests")
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=_noop_repair, verify=review
|
||||
)
|
||||
assert result.state is PreparationState.READY
|
||||
assert len(result.attempt_history) == 4 # Legacy max_repair_attempts=2 is not a loop cap.
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("defect", ["missing_unit", "failed", "empty", "stale", "missing_request"])
|
||||
async def test_approval_cannot_waive_required_execution_evidence(
|
||||
tmp_path: Path, defect: str
|
||||
) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def repair(context, checks):
|
||||
result = await _noop_repair(context, checks)
|
||||
if defect == "missing_unit":
|
||||
result.command_results = [c for c in result.command_results if c.purpose != "unit"]
|
||||
elif defect == "missing_request":
|
||||
result.command_results = [c for c in result.command_results if c.purpose != "quality"]
|
||||
else:
|
||||
check = result.command_results[0]
|
||||
if defect == "failed":
|
||||
check.status, check.exit_code = CheckStatus.FAILED, 1
|
||||
elif defect == "empty":
|
||||
check.tests_passed = 0
|
||||
else:
|
||||
check.source_digest = "a" * 64
|
||||
return result
|
||||
|
||||
request = _request(_candidate(commit))
|
||||
request.max_agent_turns = 2
|
||||
result = await prepare_fix(request, workspace, repair=repair, verify=_verified)
|
||||
assert result.state is PreparationState.BLOCKED
|
||||
assert result.final_file_manifest
|
||||
assert result.attempt_history[0].repair.gaps
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_incomplete_validation_can_be_finished_by_reviewer(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
evidence = []
|
||||
|
||||
async def repair(context, checks):
|
||||
result = await _noop_repair(context, checks)
|
||||
evidence.extend(result.command_results)
|
||||
evidence[1].status = CheckStatus.FAILED
|
||||
return result
|
||||
|
||||
async def review(context, checks):
|
||||
assert checks[1].status is CheckStatus.FAILED
|
||||
command = CommandSpec(name="existing suite", argv=checks[1].argv, purpose="unit")
|
||||
executed = record_test_execution(
|
||||
await run_command(workspace, command, network_allowed=True), command
|
||||
)
|
||||
evidence[1] = executed.model_copy(
|
||||
update={"source_digest": evidence[0].source_digest, "environment_id": "test"}
|
||||
)
|
||||
return await _verified(context, checks)
|
||||
|
||||
async def read_evidence():
|
||||
return list(evidence)
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)),
|
||||
workspace,
|
||||
repair=_noop_repair,
|
||||
repair=repair,
|
||||
verify=review,
|
||||
command_runner=runner,
|
||||
)
|
||||
assert result.state is PreparationState.BLOCKED
|
||||
assert result.final_file_manifest
|
||||
assert result.checks[0].status is CheckStatus.SKIPPED
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reviewer_concrete_defect_retries_and_reruns_tests(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def review(context, checks):
|
||||
if context.attempt == 1:
|
||||
return VerifierResult(
|
||||
decision=VerificationDecision.REJECTED,
|
||||
summary="Missing sibling guard",
|
||||
gaps=["Check the second entry point"],
|
||||
repairable=True,
|
||||
)
|
||||
assert context.feedback[0].verifier.gaps == ["Check the second entry point"]
|
||||
return await _verified(context, checks)
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=_noop_repair, verify=review
|
||||
evidence_reader=read_evidence,
|
||||
)
|
||||
assert result.state is PreparationState.READY
|
||||
assert len(result.attempt_history) == 2
|
||||
assert all(a.checks[0].tests_passed == 1 for a in result.attempt_history)
|
||||
assert result.checks[1].status is CheckStatus.PASSED
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exhausted_repair_can_be_validated_from_saved_work(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def repair(context, checks):
|
||||
outcome = await _noop_repair(context, checks)
|
||||
outcome.status = RepairStatus.BUDGET_EXHAUSTED
|
||||
return outcome
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=repair, verify=_verified
|
||||
)
|
||||
assert result.state is PreparationState.READY
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("recover", [True, False])
|
||||
async def test_empty_completion_gets_actionable_bounded_retry(
|
||||
tmp_path: Path, recover: bool
|
||||
@pytest.mark.parametrize("stop", ["budget", "blocked", "exception", "timeout", "cancel"])
|
||||
async def test_interruptions_preserve_partial_patch_without_approval(
|
||||
tmp_path: Path, stop: str
|
||||
) -> None:
|
||||
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
cancel = False
|
||||
|
||||
async def repair(context, checks):
|
||||
if context.attempt == 2:
|
||||
assert "No changed files" in context.feedback[0].repair.gaps[0]
|
||||
if recover:
|
||||
return await _noop_repair(context, checks)
|
||||
return RepairOutcome(status=RepairStatus.COMPLETE, summary="Claimed completion")
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=repair, verify=_verified
|
||||
)
|
||||
assert result.attempts == 2
|
||||
assert result.state is (PreparationState.READY if recover else PreparationState.FAILED)
|
||||
if not recover:
|
||||
assert not result.final_file_manifest
|
||||
assert "No changed files" in result.gaps[0]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_customer_suite_cannot_be_waived_with_an_explanation(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def repair(context, checks):
|
||||
outcome = await _noop_repair(context, checks)
|
||||
outcome.test_plan.unit_tests = []
|
||||
outcome.test_plan.no_unit_tests_reason = "No existing suite was found."
|
||||
return outcome
|
||||
nonlocal cancel
|
||||
result = await _noop_repair(context, checks)
|
||||
if stop == "budget":
|
||||
result.status = RepairStatus.BUDGET_EXHAUSTED
|
||||
elif stop == "blocked":
|
||||
result.status = RepairStatus.BLOCKED
|
||||
elif stop == "exception":
|
||||
raise RuntimeError("provider error")
|
||||
elif stop == "timeout":
|
||||
await asyncio.sleep(10)
|
||||
else:
|
||||
cancel = True
|
||||
return result
|
||||
|
||||
async def review(*_args):
|
||||
pytest.fail("A reviewer cannot waive required customer unit tests")
|
||||
raise AssertionError("Stopped repair must not be approved")
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=repair, verify=review
|
||||
)
|
||||
assert result.state is PreparationState.BLOCKED
|
||||
assert result.final_file_manifest
|
||||
assert any("existing unit-test suite" in gap for gap in result.gaps)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("kind", ["repair_needed", "customer_prerequisite", "optional_follow_up"])
|
||||
async def test_controller_derives_review_action_and_resolves_repair_observations(
|
||||
tmp_path: Path, kind: str
|
||||
) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def repair(context, checks):
|
||||
outcome = await _noop_repair(context, checks)
|
||||
outcome.gaps = ["An additional deployment check could help."]
|
||||
if context.attempt == 2:
|
||||
assert context.feedback[0].verifier.gaps == ["Inspect the companion configuration."]
|
||||
return outcome
|
||||
|
||||
async def review(context, checks):
|
||||
result = await _verified(context, checks)
|
||||
# The controller must derive the outcome even if the proposed decision says verified.
|
||||
result.concerns = (
|
||||
[]
|
||||
if context.attempt == 2
|
||||
else [ReviewConcern(kind=kind, summary="Inspect the companion configuration.")]
|
||||
)
|
||||
return result
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=repair, verify=review
|
||||
)
|
||||
assert result.attempts == (2 if kind == "repair_needed" else 1)
|
||||
if kind == "customer_prerequisite":
|
||||
assert result.state is PreparationState.BLOCKED
|
||||
assert result.blocker.user_action == "Inspect the companion configuration."
|
||||
else:
|
||||
assert result.state is PreparationState.READY
|
||||
assert not result.gaps
|
||||
assert not result.verifier.gaps
|
||||
if kind == "optional_follow_up":
|
||||
assert result.verifier.notes == ["Inspect the companion configuration."]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"defect",
|
||||
[
|
||||
"missing_plan",
|
||||
"missing_file",
|
||||
"rejected_test",
|
||||
"inconclusive_review",
|
||||
"exception",
|
||||
"source_change",
|
||||
],
|
||||
)
|
||||
async def test_incomplete_work_is_retained_without_ready(tmp_path: Path, defect: str) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def repair(context, checks):
|
||||
outcome = await _noop_repair(context, checks)
|
||||
if defect == "missing_plan":
|
||||
outcome.test_plan = None
|
||||
if defect == "missing_file":
|
||||
outcome.test_plan.regression_files = ["invented.py"]
|
||||
return outcome
|
||||
|
||||
async def review(context, checks):
|
||||
if defect == "exception":
|
||||
raise RuntimeError("review provider failed")
|
||||
if defect == "source_change":
|
||||
(workspace / "app.py").write_text("bad=True\n")
|
||||
result = await _verified(context, checks)
|
||||
if defect == "rejected_test":
|
||||
result.regression_test_valid = False
|
||||
if defect == "inconclusive_review":
|
||||
result.decision = VerificationDecision.INCONCLUSIVE
|
||||
return result
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=repair, verify=review
|
||||
_request(_candidate(commit)),
|
||||
workspace,
|
||||
repair=repair,
|
||||
verify=review,
|
||||
cancelled=lambda: cancel,
|
||||
policy=PreparationPolicy(timeout_seconds=1 if stop == "timeout" else 30),
|
||||
)
|
||||
assert result.state in {PreparationState.BLOCKED, PreparationState.FAILED}
|
||||
assert result.final_file_manifest
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_mutation_returns_to_repair_instead_of_destroying_run(tmp_path: Path) -> None:
|
||||
workspace, commit = _workspace(tmp_path)
|
||||
|
||||
async def review(context, checks):
|
||||
result = await _verified(context, checks)
|
||||
if context.attempt == 1:
|
||||
(workspace / "review.tmp").write_text("temporary")
|
||||
return result
|
||||
|
||||
async def repair(context, checks):
|
||||
if context.attempt > 1:
|
||||
assert "deliverable changed" in " ".join(context.feedback[-2].repair.gaps)
|
||||
(workspace / "review.tmp").unlink()
|
||||
return await _noop_repair(context, checks)
|
||||
|
||||
result = await prepare_fix(
|
||||
_request(_candidate(commit)), workspace, repair=repair, verify=review
|
||||
)
|
||||
assert result.state is PreparationState.READY
|
||||
assert result.attempts == 2
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("output", "expected"),
|
||||
[
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue