fix: persist Daytona sandbox IDs for audit and cleanup#563
Conversation
Daytona runs now write sandbox.json immediately after sandbox creation, ensuring the provider-side sandbox ID survives interruptions. The ID is also included in result.json for completed runs. Adds a sandbox_id property to BaseSandbox (None by default, overridden in DaytonaSandbox). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| """Guards the fix from PR #555 for issue #554: Daytona sandbox IDs must | ||
| be persisted immediately after creation so interrupted runs can be | ||
| audited and cleaned up.""" |
There was a problem hiding this comment.
🟡 Regression test docstrings reference wrong PR number ("PR #555" instead of "PR #563")
Per AGENTS.md: "Regression tests must name the PR/commit they guard in the docstring." The two new regression tests (test_persist_sandbox_info_writes_sandbox_json and test_result_json_includes_sandbox_id) both state "Guards the fix from PR #555" but the actual fix (the _persist_sandbox_info function, the sandbox_id parameter, etc.) is introduced in this PR #563. PR #555 does not appear in the git history. The issue number is #554, so "PR #555" appears to be a typo/hallucination — neither matching the PR number (#563) nor the issue number (#554).
| """Guards the fix from PR #555 for issue #554: Daytona sandbox IDs must | |
| be persisted immediately after creation so interrupted runs can be | |
| audited and cleaned up.""" | |
| """Guards the fix from PR #563 for issue #554: Daytona sandbox IDs must | |
| be persisted immediately after creation so interrupted runs can be | |
| audited and cleaned up.""" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| """Guards the fix from PR #555 for issue #554: result.json should include | ||
| the sandbox_id for completed runs so post-mortem cleanup can reconcile | ||
| Daytona sandboxes against rollout artifacts.""" |
There was a problem hiding this comment.
🟡 Second regression test docstring also references wrong PR number
Same issue as the other test: test_result_json_includes_sandbox_id references "PR #555" instead of "PR #563", violating the AGENTS.md convention that regression tests must name the PR they guard.
| """Guards the fix from PR #555 for issue #554: result.json should include | |
| the sandbox_id for completed runs so post-mortem cleanup can reconcile | |
| Daytona sandboxes against rollout artifacts.""" | |
| """Guards the fix from PR #563 for issue #554: result.json should include | |
| the sandbox_id for completed runs so post-mortem cleanup can reconcile | |
| Daytona sandboxes against rollout artifacts.""" |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
sandbox.jsonimmediately after sandbox creation, ensuring the provider-side sandbox ID survives process interruptions.result.jsonalso includessandbox_idfor completed runs, linking rollout artifacts to provider-side resources.sandbox_idproperty toBaseSandbox(returnsNoneby default), overridden inDaytonaSandboxto returnself._sandbox.id.isinstance(sandbox_id, str)checks to safely handle mocked environments in tests.Closes #554
Test plan
test_persist_sandbox_info_writes_sandbox_json— verifies sandbox.json is written with correct ID and providertest_persist_sandbox_info_skips_when_no_sandbox_id— Docker backends produce no sandbox.jsontest_persist_sandbox_info_skips_when_no_rollout_dir— no crash on None rollout_dirtest_result_json_includes_sandbox_id— sandbox_id appears in result.jsontest_result_json_sandbox_id_null_for_docker— sandbox_id is null when not provided🤖 Generated with Claude Code