Skip to content

Commit 6c333cf

Browse files
committed
Guard the fourth refund site against its own failure
The infrastructure-failure branch was the one refund site still missing the guard the other three got. It differs in shape: it sits on the success path rather than in an `except`, so there is no bound exception to re-raise -- the `raise EvaluationInfrastructureError(...)` runs after the refund, and an OSError from the ledger's write preempted it entirely. The consequence is a misclassification rather than a lost signal. The sidecar maps this type to "infrastructure failure" for the agent; a bare OSError falls through to the unmapped branch and is reported as "evaluation failed: OSError". Nothing keys a retry on it, so that is the whole blast radius. Build the failure before refunding so it always exists to raise, then chain the refund's own error onto it, matching the three handlers above.
1 parent 77f119b commit 6c333cf

2 files changed

Lines changed: 80 additions & 8 deletions

File tree

vero/src/vero/evaluation/engine.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -467,18 +467,29 @@ async def _execute_record(
467467
None,
468468
)
469469
if infrastructure is not None:
470+
# Build the failure before refunding so the refund cannot preempt it:
471+
# the sidecar maps this type to "infrastructure failure" for the
472+
# agent, and a bare OSError from the ledger's write would arrive
473+
# instead as an unmapped "evaluation failed: OSError".
474+
failure = EvaluationInfrastructureError(record.id, infrastructure.message)
470475
if charged:
471476
# Shield: a cancellation racing this infrastructure-failure
472477
# refund must not leak the reservation's budget.
473-
await asyncio.shield(
474-
self.budget_ledger.refund(
475-
backend_id,
476-
request.evaluation_set,
477-
cost,
478-
principal,
478+
try:
479+
await asyncio.shield(
480+
self.budget_ledger.refund(
481+
backend_id,
482+
request.evaluation_set,
483+
cost,
484+
principal,
485+
)
479486
)
480-
)
481-
raise EvaluationInfrastructureError(record.id, infrastructure.message)
487+
except Exception as refund_error:
488+
# Same guard as the three handlers above, for the same
489+
# reason: chain the refund's own failure and re-raise the
490+
# original, as BudgetStore.refund already does.
491+
raise failure from refund_error
492+
raise failure
482493
return record, decision
483494

484495
async def _record(self, record: EvaluationRecord) -> None:

vero/tests/test_v05_evaluation_runtime.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
CaseRange,
2323
CaseResult,
2424
CaseStatus,
25+
DiagnosticSeverity,
2526
DisclosureLevel,
2627
EvaluationAccessPolicy,
2728
EvaluationAuthorization,
@@ -31,8 +32,10 @@
3132
EvaluationDatabase,
3233
EvaluationDefinition,
3334
EvaluationDeniedError,
35+
EvaluationDiagnostic,
3436
EvaluationEngine,
3537
EvaluationExecutionError,
38+
EvaluationInfrastructureError,
3639
EvaluationLimits,
3740
EvaluationPlan,
3841
EvaluationPrincipal,
@@ -848,6 +851,64 @@ async def failing_refund(*args, **kwargs):
848851
assert isinstance(raised.value.__cause__, OSError)
849852

850853

854+
@pytest.mark.asyncio
855+
async def test_engine_refund_failure_preserves_the_infrastructure_error(
856+
tmp_path: Path, monkeypatch
857+
):
858+
"""The infrastructure-failure refund must not substitute its own OSError.
859+
860+
This path builds its exception after the refund rather than inside an
861+
`except`, so a failing refund used to preempt the `raise` entirely. The
862+
sidecar maps EvaluationInfrastructureError to "infrastructure failure" for
863+
the agent; a bare OSError reaches the unmapped branch and is reported as
864+
"evaluation failed: OSError" instead.
865+
"""
866+
workspace = StubWorkspace(tmp_path / "repo")
867+
backend = StubBackend(
868+
report=EvaluationReport(
869+
status=EvaluationStatus.INVALID,
870+
diagnostics=[
871+
EvaluationDiagnostic(
872+
code="infrastructure_failure",
873+
message="the sandbox host went away",
874+
severity=DiagnosticSeverity.ERROR,
875+
)
876+
],
877+
),
878+
cost=EvaluationCost(cases=1),
879+
)
880+
evaluation_set = request().evaluation_set
881+
ledger = BudgetLedger(
882+
[
883+
EvaluationBudget(
884+
backend_id="default",
885+
evaluation_set_key=evaluation_set.budget_key("default"),
886+
total_runs=2,
887+
)
888+
],
889+
path=tmp_path / "budgets.json",
890+
)
891+
ledger.save()
892+
893+
async def failing_refund(*args, **kwargs):
894+
raise OSError("durable refund write failed")
895+
896+
monkeypatch.setattr(ledger, "refund", failing_refund)
897+
898+
engine = EvaluationEngine(
899+
evaluator=evaluator(tmp_path, workspace),
900+
backends=BackendRegistry({"default": backend}),
901+
database=EvaluationDatabase(id="session"),
902+
database_path=tmp_path / "database.json",
903+
budget_ledger=ledger,
904+
authorization_resolver=allow_all_evaluations,
905+
)
906+
907+
with pytest.raises(EvaluationInfrastructureError) as raised:
908+
await engine.evaluate_record(backend_id="default", request=request())
909+
assert isinstance(raised.value.__cause__, OSError)
910+
911+
851912
@pytest.mark.asyncio
852913
async def test_cancelled_evaluation_is_terminal_indexed_and_refunded(tmp_path: Path):
853914
class BlockingBackend(StubBackend):

0 commit comments

Comments
 (0)