Skip to content

fix(sandbox): finish dependency cleanup on cancellation - #4607

Merged
seratch merged 1 commit into
openai:mainfrom
hsusul:fix/sandbox-dependency-cleanup-cancellation
Aug 23, 2026
Merged

fix(sandbox): finish dependency cleanup on cancellation#4607
seratch merged 1 commit into
openai:mainfrom
hsusul:fix/sandbox-dependency-cleanup-cancellation

Conversation

@hsusul

@hsusul hsusul commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Dependencies.aclose() stopped reverse-order cleanup as soon as an owned dependency raised CancelledError. That left earlier owned resources open and cached a cancelled close task, so subsequent close attempts could not complete the skipped cleanup.

This change records the first child cancellation, finishes exactly-once cleanup for the remaining owned dependencies, clears the container's internal state, and then propagates cancellation. Normal cleanup and non-cancellation best-effort behavior are unchanged.

Test plan

  • Added a regression with two owned dependencies proving that a later dependency's cancellation does not skip the earlier resource, cancellation still propagates, and a repeated aclose() does not close either resource twice.
  • uv run pytest tests/sandbox/test_dependencies.py::test_dependencies_aclose_finishes_owned_cleanup_before_propagating_cancellation -q — 1 passed; repeated 10 times with the same result.
  • uv run pytest tests/sandbox/test_dependencies.py -q — 20 passed.
  • make typecheck — mypy passed for 307 source files; Pyright reported 0 errors.
  • .agents/skills/code-change-verification/scripts/run.sh — format and lint passed; the broad suite completed with 9085 passed, 28 skipped, and 1 pre-existing failure. tests/sandbox/test_run_cwd.py::test_python_skill_uses_absolute_root_from_nested_workdir fails with the same missing output file on clean upstream 23346799 and is unrelated to this two-file cleanup change.
  • git diff --check — passed.

No public API, serialization format, or documentation changes.

Issue number

None filed; reproduced deterministically with local owned-resource fakes.

Checks

  • I've added new tests, if relevant
  • I've run .agents/skills/code-change-verification/scripts/run.sh
  • I've confirmed all verification steps pass
  • If using Codex, I've run /review before submitting this PR

@seratch
seratch enabled auto-merge (squash) August 23, 2026 22:07
@seratch
seratch merged commit 72b2c67 into openai:main Aug 23, 2026
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants