fix(harness): bind sandbox to per-call RuntimeContext to prevent concurrent-call corruption

agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystem.java

@@ -29,35 +29,53 @@
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * A {@link BaseSandboxFilesystem} that delegates execution to a live {@link Sandbox}.
  *
- * <p>Stable proxy created at agent build time; a fresh {@link Sandbox} is injected on each call
- * via the volatile {@code sandbox} field by {@link
- * io.agentscope.harness.agent.middleware.SandboxLifecycleMiddleware}.
+ * <p>Stable proxy created at agent build time. The active {@link Sandbox} for an operation is
+ * resolved <b>per call</b> from the {@link RuntimeContext} (bound by {@link
+ * io.agentscope.harness.agent.middleware.SandboxLifecycleMiddleware} via {@code
+ * ctx.put(Sandbox.class, ...)}), so concurrent calls on the same agent instance each see their
+ * own sandbox.
+ *
+ * <p>The agent-level {@link SandboxAware} slot is kept only as a fallback for internal paths
+ * that operate without a per-call context (e.g. {@code RuntimeContext.empty()} maintenance
+ * writes); it must not be relied on when calls may run concurrently.
  */
 public class SandboxBackedFilesystem extends BaseSandboxFilesystem implements SandboxAware {
 
     private static final Logger log = LoggerFactory.getLogger(SandboxBackedFilesystem.class);
 
     private final String fsId;
-    private volatile Sandbox sandbox;
+
+    /** Fallback slot for context-less paths; compare-and-set cleared to avoid cross-call races. */
+    private final AtomicReference<Sandbox> fallbackSandbox = new AtomicReference<>();
 
     public SandboxBackedFilesystem() {
         this.fsId = "sandbox-" + UUID.randomUUID().toString().substring(0, 8);
     }
 
     @Override
     public void setSandbox(Sandbox sandbox) {
-        this.sandbox = sandbox;
+        this.fallbackSandbox.set(sandbox);
     }
 
     @Override
     public Sandbox getSandbox() {
-        return sandbox;
+        return fallbackSandbox.get();
+    }
+
+    /**
+     * Clears the fallback slot only if it still holds {@code expected}. Used by the lifecycle
+     * middleware on release so a finishing call never wipes a sandbox injected by a newer
+     * concurrent call.
+     */
+    public void clearSandbox(Sandbox expected) {
+        fallbackSandbox.compareAndSet(expected, null);
     }
 
     @Override
@@ -68,7 +86,7 @@ public String id() {
     @Override
     public ExecuteResponse execute(
             RuntimeContext runtimeContext, String command, Integer timeoutSeconds) {
-        Sandbox active = requireSandbox();
+        Sandbox active = requireSandbox(runtimeContext);
         try {
             ExecResult result = active.exec(runtimeContext, command, timeoutSeconds);
             return new ExecuteResponse(
@@ -91,7 +109,7 @@ public ExecuteResponse execute(
     @Override
     public List<FileUploadResponse> uploadFiles(
             RuntimeContext runtimeContext, List<Map.Entry<String, byte[]>> files) {
-        Sandbox active = requireSandbox();
+        Sandbox active = requireSandbox(runtimeContext);
         List<FileUploadResponse> results = new ArrayList<>(files.size());
 
         for (Map.Entry<String, byte[]> file : files) {
@@ -147,7 +165,7 @@ public List<FileUploadResponse> uploadFiles(
     @Override
     public List<FileDownloadResponse> downloadFiles(
             RuntimeContext runtimeContext, List<String> paths) {
-        Sandbox active = requireSandbox();
+        Sandbox active = requireSandbox(runtimeContext);
         List<FileDownloadResponse> results = new ArrayList<>(paths.size());
 
         for (String path : paths) {
@@ -192,8 +210,17 @@ public List<FileDownloadResponse> downloadFiles(
         return results;
     }
 
-    private Sandbox requireSandbox() {
-        Sandbox s = sandbox;
+    private Sandbox requireSandbox(RuntimeContext runtimeContext) {
+        // Per-call binding first: each in-flight call sees the sandbox bound to its own context,
+        // so concurrent calls on the same agent instance never observe each other's sandbox.
+        if (runtimeContext != null) {
+            Sandbox bound = runtimeContext.get(Sandbox.class);
+            if (bound != null) {
+                return bound;
+            }
+        }
+        // Fallback for context-less internal paths (RuntimeContext.empty() maintenance writes).
+        Sandbox s = fallbackSandbox.get();
         if (s == null) {
             throw new SandboxException.SandboxConfigurationException(
                     "No active sandbox — sandbox filesystem used outside of a call context");

agentscope-harness/src/main/java/io/agentscope/harness/agent/middleware/SandboxLifecycleMiddleware.java

@@ -21,7 +21,6 @@
 import io.agentscope.harness.agent.sandbox.SandboxAcquireResult;
 import io.agentscope.harness.agent.sandbox.SandboxContext;
 import io.agentscope.harness.agent.sandbox.SandboxManager;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -34,17 +33,22 @@
  *   <li>Read {@link SandboxContext} from the current {@link RuntimeContext}</li>
  *   <li>Acquire a session via {@link SandboxManager}</li>
  *   <li>Start the session (4-branch workspace init)</li>
- *   <li>Inject the live session into the {@link SandboxBackedFilesystem} proxy</li>
+ *   <li>Bind the live session to the per-call {@link RuntimeContext} (and mirror it into the
+ *       {@link SandboxBackedFilesystem} fallback slot for context-less internal paths)</li>
  * </ol>
  *
  * <h2>doFinally</h2>
  * <ol>
  *   <li>Persist sandbox session state via {@link SandboxManager} and
  *       {@link io.agentscope.harness.agent.sandbox.SessionSandboxStateStore}</li>
  *   <li>Release the session via {@link SandboxManager} (stop + optional shutdown)</li>
- *   <li>Clear the session reference from the filesystem proxy</li>
+ *   <li>Unbind the session from the per-call context and CAS-clear the fallback slot</li>
  * </ol>
  *
+ * <p>All per-call state ({@link Sandbox}, {@link SandboxAcquireResult}) is carried on the
+ * per-call {@link RuntimeContext} rather than on this (agent-level, shared) middleware
+ * instance, so concurrent calls on one agent never clear or release each other's sandbox.
+ *
  * <p>Post-call failures (persist, release) are logged but do not propagate — this ensures
  * the agent call result is always returned to the caller even if sandbox cleanup fails.
  */
@@ -54,8 +58,6 @@ public class SandboxLifecycleMiddleware implements HarnessRuntimeMiddleware {
 
     private final SandboxManager sandboxManager;
     private final SandboxBackedFilesystem filesystemProxy;
-    private final AtomicReference<SandboxAcquireResult> currentAcquireResult =
-            new AtomicReference<>();
     private volatile Consumer<RuntimeContext> beforeStartCallback;
 
     public SandboxLifecycleMiddleware(
@@ -108,13 +110,19 @@ public void acquireForCall(RuntimeContext ctx) {
             Sandbox sandbox = result.getSandbox();
             try {
                 sandbox.start();
+                // Per-call binding: the filesystem proxy resolves the sandbox from the
+                // RuntimeContext, so concurrent calls each see their own sandbox. The proxy's
+                // fallback slot is mirrored only for context-less internal paths.
+                ctx.put(Sandbox.class, sandbox);
+                ctx.put(SandboxAcquireResult.class, result);
                 filesystemProxy.setSandbox(sandbox);
-                currentAcquireResult.set(result);
                 log.debug(
                         "[sandbox-mw] Acquired sandbox {}",
                         sandbox.getState() != null ? sandbox.getState().getSessionId() : "?");
             } catch (Exception e) {
-                filesystemProxy.setSandbox(null);
+                ctx.put(Sandbox.class, null);
+                ctx.put(SandboxAcquireResult.class, null);
+                filesystemProxy.clearSandbox(sandbox);
                 try {
                     sandboxManager.release(result);
                 } catch (Exception releaseErr) {
@@ -136,14 +144,22 @@ public void acquireForCall(RuntimeContext ctx) {
      * Releases the sandbox after the current call. Called from
      * {@code ReActAgent.afterAgentExecution()} to ensure cleanup for both paths.
      *
+     * <p>The acquire result is retrieved from the per-call context, so this only ever releases
+     * the sandbox that <em>this</em> call acquired — never a concurrent call's.
+     *
      * @param ctx the per-call RuntimeContext (captured at acquire time)
      */
     public void releaseForCall(RuntimeContext ctx) {
-        SandboxAcquireResult result = currentAcquireResult.getAndSet(null);
+        if (ctx == null) {
+            return;
+        }
+        SandboxAcquireResult result = ctx.get(SandboxAcquireResult.class);
         if (result == null) {
             return;
         }
-        SandboxContext sandboxContext = ctx != null ? ctx.get(SandboxContext.class) : null;
+        ctx.put(SandboxAcquireResult.class, null);
+        ctx.put(Sandbox.class, null);
+        SandboxContext sandboxContext = ctx.get(SandboxContext.class);
         try {
             sandboxManager.persistState(result, sandboxContext, ctx);
         } catch (Exception e) {
@@ -155,6 +171,6 @@ public void releaseForCall(RuntimeContext ctx) {
             log.warn("[sandbox-mw] Failed to release sandbox session: {}", e.getMessage(), e);
         }
         result.getLease().close();
-        filesystemProxy.setSandbox(null);
+        filesystemProxy.clearSandbox(result.getSandbox());
     }
 }

agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystemTest.java

@@ -143,6 +143,36 @@ void uploadFiles_reportsNativeTransferFailure() {
         assertEquals("transfer down", responses.get(0).error());
     }
 
+    @Test
+    void execute_prefersContextBoundSandboxOverFallbackSlot() {
+        SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem();
+        FakeSandbox fallback = new FakeSandbox(new ExecResult(0, "fallback", "", false));
+        FakeSandbox bound = new FakeSandbox(new ExecResult(0, "bound", "", false));
+        filesystem.setSandbox(fallback);
+        RuntimeContext ctx = RuntimeContext.empty();
+        ctx.put(Sandbox.class, bound);
+
+        filesystem.execute(ctx, "whoami", 5);
+
+        assertEquals("whoami", bound.lastCommand);
+        assertEquals(null, fallback.lastCommand);
+    }
+
+    @Test
+    void clearSandbox_onlyClearsWhenSlotStillHoldsExpected() {
+        SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem();
+        FakeSandbox first = new FakeSandbox(new ExecResult(0, "", "", false));
+        FakeSandbox second = new FakeSandbox(new ExecResult(0, "", "", false));
+
+        filesystem.setSandbox(first);
+        filesystem.setSandbox(second); // newer concurrent call overwrote the slot
+        filesystem.clearSandbox(first); // finishing older call must not wipe the newer one
+        assertEquals(second, filesystem.getSandbox());
+
+        filesystem.clearSandbox(second);
+        assertEquals(null, filesystem.getSandbox());
+    }
+
     private static final class FakeTransferSandbox extends BaseFakeSandbox
             implements SandboxFileTransfer {