[andrewpark-scaleai]

What and why

A retry could lose track of a sandbox, so teardown_run never terminated it and it ran until the provider's own timeout. When a retryable step fails, _drain_in_flight cancels its in-flight siblings. A sibling that had created a sandbox and appended its record to the live context, but whose completion write had not landed yet, lost that record: the retry_resets marker was built from the stored context only, and _rebuild_context then replaced the live one. recorded_sandboxes reads only those two places, so nothing named the sandbox.

• _rolled_back_sandboxes now also reads the live context that the rebuild replaces, captured in _roll_back_span before the undo.
• It compares sandbox ids, the key recorded_sandboxes uses, instead of whole records, and lists only the ids that no surviving record names. Before, a surviving record that serialized differently in memory and in the store, such as an older stored shape or a datetime in metadata, was named as rolled back, and an env whose ids overlapped a surviving env's (the gateway builds them from shared provider state) put ids still in use into the marker. Mixed-type dict keys no longer raise inside the undo.
• teardown.env_record_ids is the one rule for which ids an env record names, shared by teardown and the marker. It ignores a sandbox_ids value that is neither an id nor a {service: id} map, where teardown used to raise.
• The step-journal TLA+ model in tst/formal/step_journal gains ghost sandbox state and two invariants: every sandbox a run creates is recorded when the run ends (NoSandboxLeaks), and the marker never names a sandbox that is still recorded as deployed (NoMarkerNamesLive). The model found the leak before the real-code test reproduced it.

How it was tested

• tst/unit/task/retry_sandbox_teardown_test.py runs the real DeploySandboxTaskStep, Task.run and the SQLite store, and holds the sibling's completion write until the undo lands. Before the fix it fails with assert ['sb-2'] == ['sb-1', 'sb-2']. The first commit adds it as a strict xfail. It also asserts the marker names only the rolled-back sandbox.
• New cases in tst/unit/task_step/retry_test.py and tst/unit/task/teardown_test.py cover record shapes that differ between memory and store, overlapping env ids, and malformed sandbox_ids values. Each fails on the previous code and passes on the fix.
• tst/formal/step_journal/check.sh: single and takeover hold. check.sh mutations: every line ok, including MarkerFromStoreOnly (the old code) breaking NoSandboxLeaks and MarkerNamesSurvivors breaking NoMarkerNamesLive.
• pytest tst/unit/task tst/unit/task_step tst/unit/runner tst/unit/cli: 3037 passed. make lint passes.

Confidence Score: 5/5

The PR appears safe to merge; no actionable issues were found.

Summary

Keeps sandboxes from cancelled sibling steps discoverable after a retry rebuilds the context.

• Retry cleanup keeps track of sandboxes created by cancelled siblings.
• Env records use one safe rule to identify their sandbox IDs.

No actionable issues found. Tests and model checks were not run during this review.

Diagram

sequenceDiagram
    participant S as Sibling deploy step
    participant T as Task
    participant D as Store
    participant C as Teardown
    S->>T: Append sandbox to live context
    S->>D: Start completion write
    T->>S: Cancel and await sibling
    T->>T: Snapshot live context
    T->>D: Undo retry span
    D->>D: Mark removed IDs from stored and live records
    D-->>T: Rebuilt context with retry marker
    T->>C: Finished context
    C->>C: Find live and rolled-back sandboxes

Loading

_{Reviews (1) · Last reviewed commit: "fix(task): name a cancelled sibling's sa..." · Reviewed by Greptile}