Skip to content

Commit df7e1fe

Browse files
End a superseded run at its step, and keep the refusal over a cleanup failure
A step whose completion a later attempt refused now ends the run even when the step tolerates failure or has a retry budget: neither can make the run's writes land. A retry was already stopped by the fenced undo; a tolerant step let the superseded run carry on to its dependents. If deleting the refused step's journal row also fails, the refusal is still what the caller gets. The orphan row is harmless, since replay drops a step with no completion. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent bea62bb commit df7e1fe

4 files changed

Lines changed: 62 additions & 4 deletions

File tree

‎src/agent_env/task/store.py‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -727,10 +727,17 @@ def _mutate(doc: dict) -> UpdateSpec | None:
727727
if result is None and journaled is not None:
728728
# The completion never landed, so the entry describes nothing in the document.
729729
# Delete at the exact seq, so a re-record that replaced the row keeps its own.
730-
store.delete(
731-
TASK_STEP_JOURNAL_COLLECTION,
732-
Filter.of(instance_id=instance_id, step_id=step_id, seq=seq),
733-
)
730+
try:
731+
store.delete(
732+
TASK_STEP_JOURNAL_COLLECTION,
733+
Filter.of(instance_id=instance_id, step_id=step_id, seq=seq),
734+
)
735+
except Exception:
736+
if not declined:
737+
raise
738+
# Replay drops a row with no completion, so the refusal, not the cleanup, is what the caller must hear.
739+
logger.warning("Failed to delete step %s's journal row for instance_id=%s after its completion was "
740+
"refused", step_id, instance_id, exc_info=True)
734741
if result is None and declined:
735742
error = SupersededAttemptError(instance_id, step_id, declined[0])
736743
logger.warning("%s", error)

‎src/agent_env/task/task.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,12 @@ async def _drain_in_flight() -> set[str]:
678678
_unblock_dependents(step_id)
679679
continue
680680
step_obj = state.step_by_id[step_id]
681+
if isinstance(exc, _store.SupersededAttemptError):
682+
# A later attempt owns the instance: no retry or tolerance makes this run's writes land.
683+
logger.error("%sStep %s failed (superseded): %s", log_prefix, step_id, exc)
684+
if first_error is None:
685+
first_error, first_error_step_id = exc, step_id
686+
continue
681687
retry_cfg = getattr(step_obj, "retry_config", None)
682688
if retry_cfg is not None and retry_counts.get(step_id, 0) < retry_cfg.max_retries:
683689
retryable.append((step_obj, retry_cfg, exc))

‎tst/unit/task/step_journal_test.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,30 @@ def journal_then_reregister(docs, instance_id, step_id, ops, status, **kw):
521521
assert [e["step_id"] for e in _journal(store, "i")] == ["__seed__"] # orphan row cleaned up
522522

523523

524+
def test_a_refused_completion_is_raised_even_when_its_journal_row_cannot_be_removed(store, monkeypatch):
525+
# The orphan row is harmless (replay drops a step with no completion); losing the refusal is not.
526+
store.upsert_instance(instance_id="i", task_id="t", task_version=1, total_steps=3)
527+
seed_task_instance_context("i", TaskStepContext(metadata={"base": 1}))
528+
real_journal, real_delete = store._journal_step, store._doc_store.delete
529+
530+
def journal_then_reregister(docs, instance_id, step_id, ops, status, **kw):
531+
out = real_journal(docs, instance_id, step_id, ops, status, **kw)
532+
monkeypatch.setattr(store, "_journal_step", real_journal) # fire once
533+
store.upsert_instance(instance_id="i", task_id="t", task_version=1, total_steps=3)
534+
return out
535+
536+
def journal_delete_fails(collection, *args, **kwargs):
537+
if collection == TASK_STEP_JOURNAL_COLLECTION:
538+
raise RuntimeError("store unavailable")
539+
return real_delete(collection, *args, **kwargs)
540+
541+
monkeypatch.setattr(store, "_journal_step", journal_then_reregister)
542+
monkeypatch.setattr(store._doc_store, "delete", journal_delete_fails)
543+
with pytest.raises(SupersededAttemptError, match="re-registered"):
544+
_record(store, "i", "s0", _ops({"base": 1}, {"base": 1, "a": 1}), total=3)
545+
assert _doc(store, "i")["completed_steps"] == []
546+
547+
524548
def test_the_rerecord_flag_replay_uses_comes_from_the_winning_cas(store, monkeypatch):
525549
# Two writers both journal rerecorded=False, but the CAS landing second unions with
526550
# move_to_end=True — so the flag replay trusts lives on the instance, written in that CAS.

‎tst/unit/task_step/retry_test.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,6 +992,27 @@ async def test_an_attempt_a_re_register_superseded_writes_nothing_to_the_run(sto
992992
assert (d["status"], d["completed_steps"], d["context"]["metadata"]) == ("running", [], {})
993993

994994

995+
@pytest.mark.parametrize("leniency", ["tolerant", "retrying"])
996+
@pytest.mark.asyncio
997+
async def test_a_superseded_run_stops_even_at_a_step_that_tolerates_or_retries_failure(store, leniency):
998+
# Neither tolerating the step nor rolling it back can make this run's writes land, so s1 must not run.
999+
gate = asyncio.Event()
1000+
s0 = _Gated("s0", gate)
1001+
if leniency == "tolerant":
1002+
s0.fail_task_on_error = False
1003+
else:
1004+
s0.retry_config = RetryConfig(retry_from_step_id="s0")
1005+
superseded = asyncio.create_task(Task(id="t", version=1, steps=[s0, _Gated("s1", gate, fails=True)]).run(instance_id="i"))
1006+
while _doc(store, "i") is None:
1007+
await asyncio.sleep(0)
1008+
store.upsert_instance("i", "t", 1, 2)
1009+
gate.set()
1010+
with pytest.raises(SupersededAttemptError, match="'s0'"): # not s1's "superseded boom": s1 never ran
1011+
await superseded
1012+
d = _doc(store, "i")
1013+
assert (d["status"], d["completed_steps"], d["context"]["metadata"]) == ("running", [], {})
1014+
1015+
9951016
@pytest.mark.asyncio
9961017
async def test_a_cancelled_run_is_recorded_cancelled(store):
9971018
run = asyncio.create_task(Task(id="t", version=1, steps=[_Gated("s0", asyncio.Event())]).run(instance_id="i"))

0 commit comments

Comments
 (0)