From 77fc8c3293abacf91ee8ef6e9c8d31719a992a43 Mon Sep 17 00:00:00 2001 From: xjx <2869418079@qq.com> Date: Mon, 27 Jul 2026 22:47:36 +0800 Subject: [PATCH] fix(sandbox): rebind RemoteSnapshotClient on state deserialize Default deserializeState(json, snapshotSpec) ignored the snapshot spec, so non-Docker sandbox clients left RemoteSandboxSnapshot unbound after JSON round-trip. Move rebinding into the interface default and cover it with unit tests. Fixes #2303 --- .../harness/agent/sandbox/SandboxClient.java | 33 +++- .../impl/docker/DockerSandboxClient.java | 27 --- ...SandboxClientRemoteSnapshotRebindTest.java | 161 ++++++++++++++++++ 3 files changed, 193 insertions(+), 28 deletions(-) create mode 100644 agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/SandboxClientRemoteSnapshotRebindTest.java diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxClient.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxClient.java index a15f61e515..8671fab77a 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxClient.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxClient.java @@ -15,6 +15,9 @@ */ package io.agentscope.harness.agent.sandbox; +import io.agentscope.harness.agent.sandbox.snapshot.RemoteSandboxSnapshot; +import io.agentscope.harness.agent.sandbox.snapshot.RemoteSnapshotSpec; +import io.agentscope.harness.agent.sandbox.snapshot.SandboxSnapshot; import io.agentscope.harness.agent.sandbox.snapshot.SandboxSnapshotSpec; /** @@ -42,7 +45,35 @@ public interface SandboxClient { SandboxState deserializeState(String json); + /** + * Deserializes sandbox state and rebinds a {@link + * io.agentscope.harness.agent.sandbox.snapshot.RemoteSnapshotClient} when the given snapshot + * spec is a {@link RemoteSnapshotSpec}. + * + *

{@link RemoteSandboxSnapshot} only persists its {@code id} across JSON serialization; the + * client must be re-injected from the live {@link RemoteSnapshotSpec} on resume. + */ default SandboxState deserializeState(String json, SandboxSnapshotSpec snapshotSpec) { - return deserializeState(json); + SandboxState state = deserializeState(json); + rebindRemoteSnapshot(state, snapshotSpec); + return state; + } + + /** + * Rebinds {@link RemoteSandboxSnapshot} with the client from {@link RemoteSnapshotSpec}. + * + *

No-op when the spec is not remote, the snapshot is missing/non-remote, or the snapshot + * id is null. + */ + static void rebindRemoteSnapshot(SandboxState state, SandboxSnapshotSpec snapshotSpec) { + if (state == null || !(snapshotSpec instanceof RemoteSnapshotSpec remoteSnapshotSpec)) { + return; + } + SandboxSnapshot snapshot = state.getSnapshot(); + if (!(snapshot instanceof RemoteSandboxSnapshot) || snapshot.getId() == null) { + return; + } + state.setSnapshot( + new RemoteSandboxSnapshot(remoteSnapshotSpec.getClient(), snapshot.getId())); } } diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxClient.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxClient.java index a32ba4df2b..7a14f1278f 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxClient.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxClient.java @@ -22,9 +22,6 @@ import io.agentscope.harness.agent.sandbox.SandboxState; import io.agentscope.harness.agent.sandbox.WorkspaceSpec; import io.agentscope.harness.agent.sandbox.json.HarnessSandboxJacksonModule; -import io.agentscope.harness.agent.sandbox.snapshot.RemoteSandboxSnapshot; -import io.agentscope.harness.agent.sandbox.snapshot.RemoteSnapshotSpec; -import io.agentscope.harness.agent.sandbox.snapshot.SandboxSnapshot; import io.agentscope.harness.agent.sandbox.snapshot.SandboxSnapshotSpec; import java.util.UUID; import org.slf4j.Logger; @@ -133,28 +130,4 @@ public SandboxState deserializeState(String json) { "Failed to deserialize Docker sandbox state", e); } } - - @Override - public SandboxState deserializeState(String json, SandboxSnapshotSpec snapshotSpec) { - try { - SandboxState state = objectMapper.readValue(json, SandboxState.class); - rebindRemoteSnapshot(state, snapshotSpec); - return state; - } catch (Exception e) { - throw new SandboxException.SandboxConfigurationException( - "Failed to deserialize Docker sandbox state", e); - } - } - - private static void rebindRemoteSnapshot(SandboxState state, SandboxSnapshotSpec snapshotSpec) { - if (!(snapshotSpec instanceof RemoteSnapshotSpec remoteSnapshotSpec)) { - return; - } - SandboxSnapshot snapshot = state.getSnapshot(); - if (!(snapshot instanceof RemoteSandboxSnapshot)) { - return; - } - state.setSnapshot( - new RemoteSandboxSnapshot(remoteSnapshotSpec.getClient(), snapshot.getId())); - } } diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/SandboxClientRemoteSnapshotRebindTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/SandboxClientRemoteSnapshotRebindTest.java new file mode 100644 index 0000000000..d371124614 --- /dev/null +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/SandboxClientRemoteSnapshotRebindTest.java @@ -0,0 +1,161 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.harness.agent.sandbox; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.agentscope.harness.agent.sandbox.impl.docker.DockerSandboxState; +import io.agentscope.harness.agent.sandbox.json.HarnessSandboxJacksonModule; +import io.agentscope.harness.agent.sandbox.snapshot.RemoteSandboxSnapshot; +import io.agentscope.harness.agent.sandbox.snapshot.RemoteSnapshotClient; +import io.agentscope.harness.agent.sandbox.snapshot.RemoteSnapshotSpec; +import io.agentscope.harness.agent.sandbox.snapshot.SandboxSnapshot; +import io.agentscope.harness.agent.sandbox.snapshot.SandboxSnapshotSpec; +import java.io.InputStream; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Reproduces and guards against remote snapshot client loss after JSON deserialization + * when a {@link SandboxClient} only implements the single-arg {@code deserializeState}. + * + *

See #2303. + */ +class SandboxClientRemoteSnapshotRebindTest { + + @Test + @DisplayName( + "default deserializeState(json, spec) must rebind RemoteSnapshotClient so" + + " isRestorable() works") + void defaultTwoArgDeserializeRebindsRemoteClient() throws Exception { + ObjectMapper mapper = + new ObjectMapper() + .findAndRegisterModules() + .registerModule(new HarnessSandboxJacksonModule()); + + // Simulates AgentRun / Daytona / E2B / Kubernetes clients: only single-arg deserialize. + SandboxClient client = new SingleArgDeserializeClient(mapper); + + DockerSandboxState original = new DockerSandboxState(); + original.setSessionId("remote-session-rebind"); + original.setSnapshot(new RemoteSandboxSnapshot(new FakeRemoteSnapshotClient(), "snap-x")); + + String json = client.serializeState(original); + RemoteSnapshotSpec spec = new RemoteSnapshotSpec(new FakeRemoteSnapshotClient()); + + SandboxState restored = client.deserializeState(json, spec); + SandboxSnapshot snapshot = restored.getSnapshot(); + + assertInstanceOf(RemoteSandboxSnapshot.class, snapshot); + assertEquals("snap-x", snapshot.getId()); + assertTrue(snapshot.isRestorable()); + } + + @Test + @DisplayName("single-arg deserialize leaves RemoteSnapshotClient unbound (bug evidence)") + void singleArgDeserializeLeavesClientUnbound() throws Exception { + ObjectMapper mapper = + new ObjectMapper() + .findAndRegisterModules() + .registerModule(new HarnessSandboxJacksonModule()); + + SandboxClient client = new SingleArgDeserializeClient(mapper); + + DockerSandboxState original = new DockerSandboxState(); + original.setSessionId("remote-session-unbound"); + original.setSnapshot(new RemoteSandboxSnapshot(new FakeRemoteSnapshotClient(), "snap-y")); + + String json = client.serializeState(original); + SandboxState restored = client.deserializeState(json); + + SandboxException.SnapshotException ex = + assertThrows( + SandboxException.SnapshotException.class, + () -> restored.getSnapshot().isRestorable()); + assertInstanceOf(IllegalStateException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("RemoteSnapshotClient is not bound")); + } + + /** + * Minimal client that only overrides the single-arg deserialize path — same shape as + * AgentRunSandboxClient / DaytonaSandboxClient / E2bSandboxClient / KubernetesSandboxClient. + */ + private static final class SingleArgDeserializeClient + implements SandboxClient { + + private final ObjectMapper objectMapper; + + private SingleArgDeserializeClient(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + } + + @Override + public Sandbox create( + WorkspaceSpec workspaceSpec, + SandboxSnapshotSpec snapshotSpec, + SandboxClientOptions options) { + throw new UnsupportedOperationException(); + } + + @Override + public Sandbox resume(SandboxState state) { + throw new UnsupportedOperationException(); + } + + @Override + public void delete(Sandbox sandbox) {} + + @Override + public String serializeState(SandboxState state) { + try { + return objectMapper.writeValueAsString(state); + } catch (Exception e) { + throw new SandboxException.SandboxConfigurationException( + "Failed to serialize sandbox state", e); + } + } + + @Override + public SandboxState deserializeState(String json) { + try { + return objectMapper.readValue(json, SandboxState.class); + } catch (Exception e) { + throw new SandboxException.SandboxConfigurationException( + "Failed to deserialize sandbox state", e); + } + } + } + + private static final class FakeRemoteSnapshotClient implements RemoteSnapshotClient { + + @Override + public void upload(String snapshotId, InputStream data) {} + + @Override + public InputStream download(String snapshotId) { + return InputStream.nullInputStream(); + } + + @Override + public boolean exists(String snapshotId) { + return true; + } + } +}