[PR #100] [CLOSED] fix(orchestrator): restore pause state after allocation failure #178

New issue

Closed

opened 2026-10-29 14:20:42 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:20:42 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/100
Author: @morluto
Created: 7/31/2026
Status: ❌ Closed

Base: main ← Head: fix/pause-allocation-rollback

📝 Commits (2)

89618d8 fix(orchestrator): restore pause state after allocation failure
a465e71 test(orchestrator): prove pause rollback invariants

📊 Changes

2 files changed (+236 additions, -4 deletions)

View changed files

📝 src/orchestrator/service.rs (+48 -2)
📝 src/orchestrator/tests.rs (+188 -2)

📄 Description

What does this PR do?

Restores a running sandbox when paused-artifact-root allocation fails before the backend pause begins.

Previously, pause_sandbox_inner detached the runtime handle and proxy route and then propagated an allocation error with ?. That skipped the rollback used by later pause failure paths, leaving metadata in Pausing while the live runtime was no longer routable.

The failure path now restores the runtime handle and proxy route, transitions metadata from Pausing back to Running, and releases the temporary PausedSandbox image ownership before returning the allocation error. If the metadata transition also fails, the returned internal error preserves both failures.

                         allocate artifact root
Running -> Pausing -> detach handle + route --------X
                                                     |
                                                     v
                  release paused refs <- Running <- restore route + handle
                                                     |
                                                     v
                                          return allocation error

Closes #99.

How was it tested?

Added pause_artifact_root_allocation_failure_restores_running_for_retry, which deterministically fails the first artifact-root allocation and verifies:
- metadata returns to Running with no paused state;
- the runtime handle and ready proxy route are restored;
- temporary paused-image ownership is released;
- no paused state is persisted for the failed attempt; and
- a second pause succeeds and reaches Paused.
cargo test -p agentenv --lib orchestrator::service::tests::pause_artifact_root_allocation_failure_restores_running_for_retry -- --exact --nocapture
cargo fmt --all -- --check
git diff --check main...HEAD

Compatibility

No API, persistence format, or successful pause behavior changes. This only changes rollback after an artifact-root allocation error that occurs before SandboxBackend::pause is called.

Suggested review order

Review the allocation error branch in src/orchestrator/service.rs.
Review the deterministic failure and retry assertions in src/orchestrator/tests.rs.

_{🔄 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/100 **Author:** [@morluto](https://github.com/morluto) **Created:** 7/31/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/pause-allocation-rollback` --- ### 📝 Commits (2) - [`89618d8`](https://github.com/kvcache-ai/AgentENV/commit/89618d8278dc5a80cb6a6a02f9e6a42481172fca) fix(orchestrator): restore pause state after allocation failure - [`a465e71`](https://github.com/kvcache-ai/AgentENV/commit/a465e712d599ee2436577f4fd6d525406e94034d) test(orchestrator): prove pause rollback invariants ### 📊 Changes **2 files changed** (+236 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/orchestrator/service.rs` (+48 -2) 📝 `src/orchestrator/tests.rs` (+188 -2) </details> ### 📄 Description ## What does this PR do? Restores a running sandbox when paused-artifact-root allocation fails before the backend pause begins. Previously, `pause_sandbox_inner` detached the runtime handle and proxy route and then propagated an allocation error with `?`. That skipped the rollback used by later pause failure paths, leaving metadata in `Pausing` while the live runtime was no longer routable. The failure path now restores the runtime handle and proxy route, transitions metadata from `Pausing` back to `Running`, and releases the temporary `PausedSandbox` image ownership before returning the allocation error. If the metadata transition also fails, the returned internal error preserves both failures. ```text allocate artifact root Running -> Pausing -> detach handle + route --------X | v release paused refs <- Running <- restore route + handle | v return allocation error ``` Closes #99. ## How was it tested? - Added `pause_artifact_root_allocation_failure_restores_running_for_retry`, which deterministically fails the first artifact-root allocation and verifies: - metadata returns to `Running` with no paused state; - the runtime handle and ready proxy route are restored; - temporary paused-image ownership is released; - no paused state is persisted for the failed attempt; and - a second pause succeeds and reaches `Paused`. - `cargo test -p agentenv --lib orchestrator::service::tests::pause_artifact_root_allocation_failure_restores_running_for_retry -- --exact --nocapture` - `cargo fmt --all -- --check` - `git diff --check main...HEAD` ## Compatibility No API, persistence format, or successful pause behavior changes. This only changes rollback after an artifact-root allocation error that occurs before `SandboxBackend::pause` is called. ## Suggested review order 1. Review the allocation error branch in `src/orchestrator/service.rs`. 2. Review the deterministic failure and retry assertions in `src/orchestrator/tests.rs`. --- <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:42 +00:00