[PR #64] [MERGED] fix(sandbox): resume sandbox upon recoverable pause failure #149

Closed
opened 2026-10-29 14:20:16 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/64
Author: @LSX-s-Software
Created: 7/29/2026
Status: ✅ Merged
Merged: 7/30/2026
Merged by: @yingdi-shan

Base: main ← Head: fix/recoverable-pause-failure


📝 Commits (1)

  • 966806c fix(sandbox): resume sandbox upon recoverable pause failure

📊 Changes

4 files changed (+105 additions, -12 deletions)

View changed files

📝 src/orchestrator/tests.rs (+69 -1)
📝 src/sandbox/backend.rs (+3 -0)
📝 src/sandbox/firecracker/sandbox.rs (+19 -9)
📝 src/sandbox/mock.rs (+14 -2)

📄 Description

What

Ensure SandboxBackend::pause() returns a recoverable error only after the sandbox has been restored to a running state.

For the Firecracker backend, a recoverable pause failure now triggers an in-place resume before the error is returned. If recovery also fails, the error is promoted to terminal so the orchestrator stops and removes the sandbox instead of publishing a frozen VM as Running.

Why

Firecracker is paused before snapshot artifacts are fully created. A later snapshot-processing failure could therefore leave the VM paused while the orchestrator restored its handle, proxy route, and Running metadata. The sandbox appeared healthy and routable but could not make progress.

Keeping the recovery guarantee at the backend boundary lets the orchestrator retain its simple error handling without depending on backend-specific intermediate states.

Closes #41

Scope and non-goals

Included:

  • Define the recoverable pause() contract at the sandbox backend boundary.
  • Resume Firecracker after recoverable pause failures.
  • Promote failed recovery to a terminal error with both failure causes.
  • Keep the mock backend aligned with the production contract.
  • Add focused orchestrator regression coverage.

Not included:

  • Lifecycle concurrency or delete-path refactoring.
  • Changes to the low-level FirecrackerSandbox::pause_to_dir() semantics.
  • New public APIs, configuration, or persisted state.

Design and behavior changes

backend pause failure
        |
        +-- terminal ----------------------> return Terminal
        |
        `-- recoverable --> resume backend
                              |
                              +-- success --> return original Recoverable
                              |
                              `-- failure --> return Terminal(pause + resume errors)

The orchestrator can safely restore the handle, proxy route, and Running metadata for a recoverable error because the backend now guarantees that recovery has already completed.

Compatibility and operations

  • Public API or generated protocol: N/A; no API or generated-code changes.
  • Configuration or defaults: N/A.
  • Snapshot manifest, artifact layout, or storage format: N/A.
  • Upgrade and rollback: No migration required; rollback restores the previous pause failure behavior.
  • Host requirements, permissions, ports, or dependencies: N/A.

Validation

  • make fmt (equivalent command run directly)
  • make clippy (equivalent focused command run directly)
  • make test-unit
  • Relevant Rust integration tests
  • make -C services test (required when services/ changes)
  • Generated clients/server regenerated with the documented make target
  • Documentation updated
  • Benchmarks or performance comparison completed

Commands and results:

cargo test -p agentenv --lib pause_ -- --nocapture
# 17 passed; 0 failed

cargo fmt --all -- --check
# passed

cargo clippy -p agentenv --all-targets -- -D warnings
# passed

Skipped checks and reasons:

  • Services tests were not run because services/ is unchanged.
  • Code generation and benchmarks are not applicable.

Risks and reviewer notes

N/A

Checklist

  • The PR contains one coherent change and no unrelated formatting or refactoring.
  • New behavior is covered by tests, or I explained why testing is impractical.
  • Logs and examples contain no credentials, tokens, or private registry information.
  • I did not manually edit generated code without updating its source and regenerating it.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/64 **Author:** [@LSX-s-Software](https://github.com/LSX-s-Software) **Created:** 7/29/2026 **Status:** ✅ Merged **Merged:** 7/30/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/recoverable-pause-failure` --- ### 📝 Commits (1) - [`966806c`](https://github.com/kvcache-ai/AgentENV/commit/966806cdd3cba730fb22be2903e8cce1fbf9de7f) fix(sandbox): resume sandbox upon recoverable pause failure ### 📊 Changes **4 files changed** (+105 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `src/orchestrator/tests.rs` (+69 -1) 📝 `src/sandbox/backend.rs` (+3 -0) 📝 `src/sandbox/firecracker/sandbox.rs` (+19 -9) 📝 `src/sandbox/mock.rs` (+14 -2) </details> ### 📄 Description ## What Ensure `SandboxBackend::pause()` returns a recoverable error only after the sandbox has been restored to a running state. For the Firecracker backend, a recoverable pause failure now triggers an in-place resume before the error is returned. If recovery also fails, the error is promoted to terminal so the orchestrator stops and removes the sandbox instead of publishing a frozen VM as `Running`. ## Why Firecracker is paused before snapshot artifacts are fully created. A later snapshot-processing failure could therefore leave the VM paused while the orchestrator restored its handle, proxy route, and `Running` metadata. The sandbox appeared healthy and routable but could not make progress. Keeping the recovery guarantee at the backend boundary lets the orchestrator retain its simple error handling without depending on backend-specific intermediate states. ## Related issue Closes #41 ## Scope and non-goals Included: - Define the recoverable `pause()` contract at the sandbox backend boundary. - Resume Firecracker after recoverable pause failures. - Promote failed recovery to a terminal error with both failure causes. - Keep the mock backend aligned with the production contract. - Add focused orchestrator regression coverage. Not included: - Lifecycle concurrency or delete-path refactoring. - Changes to the low-level `FirecrackerSandbox::pause_to_dir()` semantics. - New public APIs, configuration, or persisted state. ## Design and behavior changes ```text backend pause failure | +-- terminal ----------------------> return Terminal | `-- recoverable --> resume backend | +-- success --> return original Recoverable | `-- failure --> return Terminal(pause + resume errors) ``` The orchestrator can safely restore the handle, proxy route, and `Running` metadata for a recoverable error because the backend now guarantees that recovery has already completed. ## Compatibility and operations - Public API or generated protocol: N/A; no API or generated-code changes. - Configuration or defaults: N/A. - Snapshot manifest, artifact layout, or storage format: N/A. - Upgrade and rollback: No migration required; rollback restores the previous pause failure behavior. - Host requirements, permissions, ports, or dependencies: N/A. ## Validation - [x] `make fmt` (equivalent command run directly) - [x] `make clippy` (equivalent focused command run directly) - [x] `make test-unit` - [x] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [x] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text cargo test -p agentenv --lib pause_ -- --nocapture # 17 passed; 0 failed cargo fmt --all -- --check # passed cargo clippy -p agentenv --all-targets -- -D warnings # passed ``` Skipped checks and reasons: - Services tests were not run because `services/` is unchanged. - Code generation and benchmarks are not applicable. ## Risks and reviewer notes N/A ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-29 14:20:16 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
kvcache-ai/AgentENV#149
No description provided.