Add bounded fix verification feedback loop

This commit is contained in:
yoni 2026-09-25 21:08:38 +00:00
parent be1c2e4767
commit 9d525ade7f
4 changed files with 448 additions and 131 deletions

View file

@ -8,9 +8,12 @@ from strix.fix.contracts import (
FileManifestEntry,
FixCandidateV1,
FixEdit,
FixPreparationAttempt,
FixPreparationRequestV1,
FixPreparationResultV1,
PreparationState,
RepairOutcome,
RepairStatus,
ReproductionSpec,
SourceIdentity,
SourceIdentityKind,
@ -35,12 +38,15 @@ __all__ = [
"FileManifestEntry",
"FixCandidateV1",
"FixEdit",
"FixPreparationAttempt",
"FixPreparationRequestV1",
"FixPreparationResultV1",
"PreparationCancelledError",
"PreparationContext",
"PreparationPolicy",
"PreparationState",
"RepairOutcome",
"RepairStatus",
"ReproductionSpec",
"SourceIdentity",
"SourceIdentityKind",

View file

@ -47,6 +47,13 @@ class VerificationDecision(StrEnum):
INCONCLUSIVE = "inconclusive"
class RepairStatus(StrEnum):
COMPLETE = "complete"
BLOCKED = "blocked"
BUDGET_EXHAUSTED = "budget_exhausted"
INCOMPLETE = "incomplete"
class SourceIdentity(ContractModel):
kind: SourceIdentityKind
value: str = Field(min_length=1)
@ -154,7 +161,7 @@ class FixPreparationRequestV1(ContractModel):
repository_id: str | None = None
candidate: FixCandidateV1
checks: list[CommandSpec] = []
max_repair_attempts: int = Field(default=2, ge=1, le=5)
max_repair_attempts: int = Field(default=4, ge=1, le=4)
timeout_seconds: int = Field(default=1800, ge=30, le=14400)
network_allowed: bool = False
credentials_allowed: list[str] = []
@ -181,6 +188,23 @@ class VerifierResult(ContractModel):
gaps: list[str] = []
class RepairOutcome(ContractModel):
status: RepairStatus
summary: str = Field(min_length=1)
gaps: list[str] = []
reproduction_command: CommandSpec | None = None
turns_used: int = Field(default=0, ge=0)
class FixPreparationAttempt(ContractModel):
attempt: int = Field(ge=1)
repair: RepairOutcome
checks: list[CheckResult] = []
security_reproduction: CheckResult | None = None
verifier: VerifierResult
workspace_digest: str = Field(pattern=r"^[0-9a-f]{64}$")
class FileManifestEntry(ContractModel):
path: str
operation: Literal["add", "modify", "delete"]
@ -205,6 +229,7 @@ class FixPreparationResultV1(ContractModel):
checks: list[CheckResult] = []
security_reproduction: CheckResult | None = None
verifier: VerifierResult | None = None
attempt_history: list[FixPreparationAttempt] = []
gaps: list[str] = []
attempts: int = Field(default=0, ge=0)
elapsed_seconds: float = Field(default=0, ge=0)

View file

@ -5,11 +5,12 @@ from __future__ import annotations
import asyncio
import functools
import hashlib
import json
import os
import subprocess
import time
from collections.abc import Awaitable, Callable, Iterable
from dataclasses import dataclass
from dataclasses import dataclass, field
from pathlib import Path
from typing import Literal
@ -19,9 +20,12 @@ from strix.fix.contracts import (
CommandSpec,
FileManifestEntry,
FixCandidateV1,
FixPreparationAttempt,
FixPreparationRequestV1,
FixPreparationResultV1,
PreparationState,
RepairOutcome,
RepairStatus,
VerificationDecision,
VerifierResult,
)
@ -39,10 +43,14 @@ CancellationCheck = Callable[[], bool]
@dataclass(slots=True)
class PreparationPolicy:
max_repair_attempts: int = 2
max_repair_attempts: int = 4
timeout_seconds: int = 1800
max_output_chars: int = 20000
def __post_init__(self) -> None:
if not 1 <= self.max_repair_attempts <= 4:
raise ValueError("max_repair_attempts must be between 1 and 4")
@dataclass(slots=True)
class PreparationContext:
@ -50,11 +58,12 @@ class PreparationContext:
workspace: Path
candidate: FixCandidateV1
attempt: int = 0
feedback: list[FixPreparationAttempt] = field(default_factory=list)
RepairAgent = Callable[
[PreparationContext, list[CheckResult]],
Awaitable[None],
Awaitable[RepairOutcome | None],
]
IndependentVerifier = Callable[
[PreparationContext, list[CheckResult], CheckResult | None],
@ -306,27 +315,14 @@ async def build_git_manifest(
return entries, summary.decode(errors="replace").strip(), None
def _applied_hashes(workspace: Path, candidate: FixCandidateV1) -> dict[Path, str]:
"""Content hashes of each edited file, captured right after the draft is applied."""
applied: dict[Path, str] = {}
for edit in candidate.draft_edits:
path = (workspace / edit.file).resolve()
applied[path] = hashlib.sha256(path.read_bytes()).hexdigest()
return applied
def _change_extends_draft(
workspace: Path,
applied_sha256: dict[Path, str],
manifest: list[FileManifestEntry],
) -> bool:
"""Whether the verified change set differs from the applied draft edits."""
final_changes = {
(workspace / entry.path).resolve(): entry.resulting_sha256 for entry in manifest
}
return set(final_changes) != set(applied_sha256) or any(
resulting != applied_sha256[resolved] for resolved, resulting in final_changes.items()
)
async def _workspace_digest(workspace: Path) -> str:
manifest, _, _ = await build_git_manifest(workspace)
payload = json.dumps(
[entry.model_dump(mode="json") for entry in manifest],
sort_keys=True,
separators=(",", ":"),
).encode()
return hashlib.sha256(payload).hexdigest()
async def _verify_source(context: PreparationContext) -> bool:
@ -371,6 +367,7 @@ def _result(
manifest: list[FileManifestEntry] | None = None,
diff_summary: str = "",
artifact_ref: str | None = None,
attempt_history: list[FixPreparationAttempt] | None = None,
) -> FixPreparationResultV1:
return FixPreparationResultV1(
state=state,
@ -385,13 +382,71 @@ def _result(
checks=checks or [],
security_reproduction=reproduction,
verifier=verifier,
attempt_history=attempt_history or [],
gaps=gaps or [],
attempts=context.attempt,
elapsed_seconds=time.monotonic() - started,
)
async def prepare_fix(
def _repair_outcome(value: RepairOutcome | None) -> RepairOutcome:
if value is not None:
return value
return RepairOutcome(
status=RepairStatus.COMPLETE,
summary="The repair implementation returned control for independent evaluation.",
)
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 for result in required)
def _verification_passes(
checks: list[CheckResult],
reproduction: CheckResult | None,
verifier: VerifierResult,
) -> bool:
return (
_required_checks_pass(checks)
and reproduction is not None
and reproduction.status is CheckStatus.PASSED
and verifier.decision is VerificationDecision.VERIFIED
and verifier.security_invariant_closed
and verifier.reproduction_executed
)
def _verification_gaps(
repair_outcome: RepairOutcome,
checks: list[CheckResult],
reproduction: CheckResult | None,
verifier: VerifierResult,
) -> list[str]:
gaps = list(repair_outcome.gaps)
required = [result for result in checks if result.required]
if not required:
gaps.append("No required repository check was configured.")
gaps.extend(
f"{result.name}: required check {result.status}"
for result in required
if result.status is not CheckStatus.PASSED
)
gaps.extend(
f"{result.name}: optional check {result.status}"
for result in checks
if not result.required and result.status is not CheckStatus.PASSED
)
if reproduction is None:
gaps.append("No executable security reproduction was available.")
elif reproduction.status is not CheckStatus.PASSED:
gaps.append(f"{reproduction.name}: security reproduction {reproduction.status}")
gaps.extend(verifier.gaps)
return list(dict.fromkeys(gaps))
async def prepare_fix( # noqa: PLR0915
request: FixPreparationRequestV1,
workspace: Path,
*,
@ -417,7 +472,7 @@ async def prepare_fix(
network_allowed=request.network_allowed,
)
async def execute() -> FixPreparationResultV1: # noqa: PLR0911, PLR0912
async def execute() -> FixPreparationResultV1: # noqa: PLR0911
if cancelled():
raise PreparationCancelledError
if not await source_verifier(context):
@ -452,112 +507,144 @@ async def prepare_fix(
)
context.candidate = anchored
_apply_edits(workspace, context.candidate)
applied_sha256 = _applied_hashes(workspace, context.candidate)
checks: list[CheckResult] = []
reproduction: CheckResult | None = None
verifier: VerifierResult | None = None
attempt_history: list[FixPreparationAttempt] = []
previous_workspace_digest: str | None = None
reproduction_command = (
context.candidate.reproduction.command
if context.candidate.reproduction is not None
else None
)
for attempt in range(1, resolved_policy.max_repair_attempts + 1):
context.attempt = attempt
if cancelled():
raise PreparationCancelledError
await repair(context, checks)
repair_outcome = _repair_outcome(await repair(context, checks))
if repair_outcome.reproduction_command is not None:
reproduction_command = repair_outcome.reproduction_command
checks = [await runner(workspace, check) for check in request.checks]
if context.candidate.reproduction and context.candidate.reproduction.command:
reproduction = await runner(
workspace,
context.candidate.reproduction.command,
reproduction = (
await runner(workspace, reproduction_command)
if reproduction_command is not None
else None
)
verifier = await verify(context, checks, reproduction)
workspace_digest = await _workspace_digest(workspace)
attempt_record = FixPreparationAttempt(
attempt=attempt,
repair=repair_outcome,
checks=checks,
security_reproduction=reproduction,
verifier=verifier,
workspace_digest=workspace_digest,
)
attempt_history.append(attempt_record)
context.feedback = list(attempt_history)
gaps = _verification_gaps(repair_outcome, checks, reproduction, verifier)
gaps = list(dict.fromkeys(gaps))
if _verification_passes(checks, reproduction, verifier):
manifest, summary, artifact_ref = await manifest_builder(workspace)
if not manifest:
return _result(
context,
state=PreparationState.NEEDS_REVIEW,
reason="The prepared workspace does not contain a source change.",
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=[*gaps, "No prepared source change was produced."],
attempt_history=attempt_history,
started=started,
)
if gaps:
return _result(
context,
state=PreparationState.READY_WITH_GAPS,
reason=("The fix passed available checks, but verification gaps remain."),
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=gaps,
manifest=manifest,
diff_summary=summary,
artifact_ref=artifact_ref,
attempt_history=attempt_history,
started=started,
)
return _result(
context,
state=PreparationState.READY,
reason="The fix passed required checks and independent verification.",
checks=checks,
reproduction=reproduction,
verifier=verifier,
manifest=manifest,
diff_summary=summary,
artifact_ref=artifact_ref,
attempt_history=attempt_history,
started=started,
)
failed = any(
result.required and result.status is CheckStatus.FAILED for result in checks
)
reproduction_failed = (
reproduction is not None and reproduction.status is CheckStatus.FAILED
)
if not failed and not reproduction_failed:
break
else:
return _result(
context,
state=PreparationState.FAILED,
reason="Required checks still fail after the repair limit.",
checks=checks,
reproduction=reproduction,
started=started,
)
if repair_outcome.status is RepairStatus.BLOCKED:
manifest, summary, artifact_ref = await manifest_builder(workspace)
return _result(
context,
state=PreparationState.BLOCKED,
reason=repair_outcome.summary,
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=gaps,
manifest=manifest,
diff_summary=summary,
artifact_ref=artifact_ref,
attempt_history=attempt_history,
started=started,
)
verifier = await verify(context, checks, reproduction)
if previous_workspace_digest == workspace_digest:
manifest, summary, artifact_ref = await manifest_builder(workspace)
return _result(
context,
state=PreparationState.NEEDS_REVIEW,
reason="The repair made no repository progress after verification feedback.",
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=[*gaps, "Two repair cycles produced the same repository state."],
manifest=manifest,
diff_summary=summary,
artifact_ref=artifact_ref,
attempt_history=attempt_history,
started=started,
)
previous_workspace_digest = workspace_digest
assert verifier is not None
manifest, summary, artifact_ref = await manifest_builder(workspace)
gaps = [
result.name
for result in checks
if result.required and result.status is CheckStatus.UNAVAILABLE
]
gaps.extend(
f"{result.name}: optional check {result.status}"
for result in checks
if not result.required and result.status is not CheckStatus.PASSED
)
if _change_extends_draft(workspace, applied_sha256, manifest):
gaps.append("The verified change extends beyond the recorded draft edits.")
if reproduction is None and not verifier.reproduction_executed:
gaps.append("No executable security reproduction was available.")
elif reproduction is not None and reproduction.status is CheckStatus.UNAVAILABLE:
gaps.append(reproduction.name)
gaps.extend(verifier.gaps)
if (
verifier.decision is not VerificationDecision.VERIFIED
or not verifier.security_invariant_closed
):
return _result(
context,
state=PreparationState.NEEDS_REVIEW,
reason=verifier.summary,
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=gaps,
manifest=manifest,
diff_summary=summary,
artifact_ref=artifact_ref,
started=started,
)
if not manifest:
return _result(
context,
state=PreparationState.NEEDS_REVIEW,
reason="The prepared workspace does not contain a source change.",
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=[*gaps, "No prepared source change was produced."],
started=started,
)
if gaps:
return _result(
context,
state=PreparationState.READY_WITH_GAPS,
reason="The fix passed available checks, but required verification gaps remain.",
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=gaps,
manifest=manifest,
diff_summary=summary,
artifact_ref=artifact_ref,
started=started,
)
return _result(
context,
state=PreparationState.READY,
reason="The fix passed required checks and independent verification.",
state=PreparationState.NEEDS_REVIEW,
reason="The fix did not satisfy verification within the repair cycle limit.",
checks=checks,
reproduction=reproduction,
verifier=verifier,
gaps=[
*_verification_gaps(
attempt_history[-1].repair,
checks,
reproduction,
verifier,
),
],
manifest=manifest,
diff_summary=summary,
artifact_ref=artifact_ref,
attempt_history=attempt_history,
started=started,
)
@ -569,6 +656,7 @@ async def prepare_fix(
context,
state=PreparationState.FAILED,
reason="Fix preparation was cancelled.",
attempt_history=context.feedback,
started=started,
)
except TimeoutError:
@ -576,5 +664,6 @@ async def prepare_fix(
context,
state=PreparationState.FAILED,
reason="Fix preparation exceeded its time limit.",
attempt_history=context.feedback,
started=started,
)

View file

@ -19,6 +19,8 @@ from strix.fix.contracts import (
FixEdit,
FixPreparationRequestV1,
PreparationState,
RepairOutcome,
RepairStatus,
ReproductionSpec,
SourceIdentity,
SourceIdentityKind,
@ -67,6 +69,18 @@ def _candidate(
*,
reproduction: ReproductionSpec | None = None,
) -> FixCandidateV1:
if reproduction is None:
reproduction = ReproductionSpec(
instructions="Confirm that result returns safe.",
command=CommandSpec(
name="security reproduction",
argv=[
sys.executable,
"-c",
"from app import result; assert result() == 'safe'",
],
),
)
return FixCandidateV1(
source_identity=SourceIdentity(kind=SourceIdentityKind.COMMIT, value=commit),
security_invariant="Return a safe value.",
@ -91,7 +105,7 @@ def _candidate(
)
def _request(candidate: FixCandidateV1, *, attempts: int = 2) -> FixPreparationRequestV1:
def _request(candidate: FixCandidateV1, *, attempts: int = 4) -> FixPreparationRequestV1:
return FixPreparationRequestV1(
scan_id="scan-1",
finding_id="finding-1",
@ -113,8 +127,11 @@ def _request(candidate: FixCandidateV1, *, attempts: int = 2) -> FixPreparationR
async def _noop_repair(
_context: PreparationContext,
_checks: list[CheckResult],
) -> None:
return None
) -> RepairOutcome:
return RepairOutcome(
status=RepairStatus.COMPLETE,
summary="The draft is ready for independent evaluation.",
)
async def _verified(
@ -245,7 +262,7 @@ async def test_prepare_fix_returns_ready_with_manifest(tmp_path: Path) -> None:
@pytest.mark.asyncio
async def test_prepare_fix_demotes_ready_when_repair_exceeds_draft(
async def test_prepare_fix_allows_verified_multi_file_repairs(
tmp_path: Path,
) -> None:
workspace, commit = _workspace(tmp_path)
@ -253,8 +270,12 @@ async def test_prepare_fix_demotes_ready_when_repair_exceeds_draft(
async def widening_repair(
_context: PreparationContext,
_checks: list[CheckResult],
) -> None:
) -> RepairOutcome:
(workspace / "hardening.py").write_text("HELPER = True\n", encoding="utf-8")
return RepairOutcome(
status=RepairStatus.COMPLETE,
summary="Added the companion hardening module.",
)
result = await prepare_fix(
_request(_candidate(commit)),
@ -263,44 +284,82 @@ async def test_prepare_fix_demotes_ready_when_repair_exceeds_draft(
verify=_verified,
)
assert result.state is PreparationState.READY_WITH_GAPS
assert any("beyond the recorded draft edits" in gap for gap in result.gaps)
assert result.state is PreparationState.READY
assert {entry.path for entry in result.final_file_manifest} == {
"app.py",
"hardening.py",
}
assert result.candidate.digest() == result.candidate_digest
@pytest.mark.asyncio
async def test_prepare_fix_retries_failed_checks(tmp_path: Path) -> None:
workspace, commit = _workspace(tmp_path)
calls = 0
compile_calls = 0
async def runner(_workspace: Path, command: CommandSpec) -> CheckResult:
nonlocal calls
calls += 1
nonlocal compile_calls
if command.name == "compile":
compile_calls += 1
failed = command.name == "compile" and compile_calls == 1
return CheckResult(
name=command.name,
argv=command.argv,
status=CheckStatus.FAILED if calls == 1 else CheckStatus.PASSED,
exit_code=1 if calls == 1 else 0,
status=CheckStatus.FAILED if failed else CheckStatus.PASSED,
exit_code=1 if failed else 0,
duration_seconds=0,
required=command.required,
)
repair_calls = 0
async def repair(
_context: PreparationContext,
_checks: list[CheckResult],
) -> RepairOutcome:
nonlocal repair_calls
repair_calls += 1
if repair_calls == 2:
(workspace / "app.py").write_text(
"def result():\n return 'safe'\n# retry\n",
encoding="utf-8",
)
return RepairOutcome(
status=RepairStatus.COMPLETE,
summary="Repair cycle complete.",
)
result = await prepare_fix(
_request(_candidate(commit)),
workspace,
repair=_noop_repair,
repair=repair,
verify=_verified,
command_runner=runner,
)
assert result.state is PreparationState.READY
assert result.attempts == 2
assert calls == 2
assert compile_calls == 2
assert len(result.attempt_history) == 2
@pytest.mark.asyncio
async def test_prepare_fix_stops_at_repair_limit(tmp_path: Path) -> None:
workspace, commit = _workspace(tmp_path)
repair_calls = 0
async def repair(
_context: PreparationContext,
_checks: list[CheckResult],
) -> RepairOutcome:
nonlocal repair_calls
repair_calls += 1
with (workspace / "app.py").open("a", encoding="utf-8") as handle:
handle.write(f"# attempt {repair_calls}\n")
return RepairOutcome(
status=RepairStatus.COMPLETE,
summary="Repair cycle complete.",
)
async def runner(_workspace: Path, command: CommandSpec) -> CheckResult:
return CheckResult(
@ -313,16 +372,154 @@ async def test_prepare_fix_stops_at_repair_limit(tmp_path: Path) -> None:
)
result = await prepare_fix(
_request(_candidate(commit), attempts=2),
_request(_candidate(commit), attempts=4),
workspace,
repair=_noop_repair,
repair=repair,
verify=_verified,
command_runner=runner,
)
assert result.state is PreparationState.FAILED
assert result.state is PreparationState.NEEDS_REVIEW
assert result.attempts == 4
assert len(result.attempt_history) == 4
assert "cycle limit" in result.stop_reason
@pytest.mark.asyncio
async def test_prepare_fix_feeds_verifier_rejection_into_next_repair(
tmp_path: Path,
) -> None:
workspace, commit = _workspace(tmp_path)
repair_calls = 0
verifier_calls = 0
async def repair(
context: PreparationContext,
_checks: list[CheckResult],
) -> RepairOutcome:
nonlocal repair_calls
repair_calls += 1
if repair_calls == 2:
assert context.feedback[0].verifier.gaps == ["Harden the sibling path."]
(workspace / "sibling.py").write_text("SAFE = True\n", encoding="utf-8")
return RepairOutcome(
status=RepairStatus.COMPLETE,
summary="Repair cycle complete.",
)
async def verify(
_context: PreparationContext,
_checks: list[CheckResult],
_reproduction: CheckResult | None,
) -> VerifierResult:
nonlocal verifier_calls
verifier_calls += 1
if verifier_calls == 1:
return VerifierResult(
decision=VerificationDecision.REJECTED,
summary="A sibling path remains vulnerable.",
gaps=["Harden the sibling path."],
)
return await _verified(_context, _checks, _reproduction)
result = await prepare_fix(
_request(_candidate(commit)),
workspace,
repair=repair,
verify=verify,
)
assert result.state is PreparationState.READY
assert result.attempts == 2
assert "repair limit" in result.stop_reason
assert result.attempt_history[0].verifier.decision is VerificationDecision.REJECTED
assert result.attempt_history[1].verifier.decision is VerificationDecision.VERIFIED
@pytest.mark.asyncio
async def test_prepare_fix_stops_after_repeated_repository_state(tmp_path: Path) -> None:
workspace, commit = _workspace(tmp_path)
async def rejected(
_context: PreparationContext,
_checks: list[CheckResult],
_reproduction: CheckResult | None,
) -> VerifierResult:
return VerifierResult(
decision=VerificationDecision.REJECTED,
summary="The fix remains incomplete.",
gaps=["Change the implementation."],
)
result = await prepare_fix(
_request(_candidate(commit)),
workspace,
repair=_noop_repair,
verify=rejected,
)
assert result.state is PreparationState.NEEDS_REVIEW
assert result.attempts == 2
assert "no repository progress" in result.stop_reason
@pytest.mark.asyncio
async def test_prepare_fix_evaluates_budget_exhausted_patch(tmp_path: Path) -> None:
workspace, commit = _workspace(tmp_path)
async def exhausted(
_context: PreparationContext,
_checks: list[CheckResult],
) -> RepairOutcome:
return RepairOutcome(
status=RepairStatus.BUDGET_EXHAUSTED,
summary="The repair agent reached its turn limit.",
turns_used=40,
)
result = await prepare_fix(
_request(_candidate(commit)),
workspace,
repair=exhausted,
verify=_verified,
)
assert result.state is PreparationState.READY
assert result.attempt_history[0].repair.status is RepairStatus.BUDGET_EXHAUSTED
assert result.attempt_history[0].repair.turns_used == 40
@pytest.mark.asyncio
async def test_prepare_fix_runs_repair_proposed_reproduction(tmp_path: Path) -> None:
workspace, commit = _workspace(tmp_path)
candidate = _candidate(commit).model_copy(update={"reproduction": None})
async def repair(
_context: PreparationContext,
_checks: list[CheckResult],
) -> RepairOutcome:
return RepairOutcome(
status=RepairStatus.COMPLETE,
summary="The patch and reproduction are ready.",
reproduction_command=CommandSpec(
name="repair-proposed security reproduction",
argv=[
sys.executable,
"-c",
"from app import result; assert result() == 'safe'",
],
),
)
result = await prepare_fix(
_request(candidate),
workspace,
repair=repair,
verify=_verified,
)
assert result.state is PreparationState.READY
assert result.security_reproduction is not None
assert result.security_reproduction.name == "repair-proposed security reproduction"
@pytest.mark.asyncio