Repository navigation
fix(harness): use dynamic workspace root in middleware prompt #2194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
186a44a
827ccd4
88fc0e5
d97a79e
e69d08d
3d4d6d6
8f092d7
a905ae4
8213e99
f7da480
5e0ce9d
dea1e29
54c15d3
e6bf833
d0a3a37
4b1f2c9
4f79737
ba68ece
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ public class AgentRunFilesystemSpec extends SandboxFilesystemSpec { | |
| private SandboxClient<?> client; | ||
| private final AgentRunSandboxClientOptions options = new AgentRunSandboxClientOptions(); | ||
| private SandboxSnapshotSpec snapshotSpec = new NoopSnapshotSpec(); | ||
| private WorkspaceSpec defaultWorkspaceSpec = new WorkspaceSpec(); | ||
| private WorkspaceSpec defaultWorkspaceSpec = createDefaultWorkspaceSpec(); | ||
|
|
||
| public AgentRunFilesystemSpec client(SandboxClient<?> client) { | ||
| this.client = client; | ||
|
|
@@ -86,7 +86,11 @@ public AgentRunFilesystemSpec addOssMount(AgentRunOssMountConfig mount) { | |
| } | ||
|
|
||
| public AgentRunFilesystemSpec workspaceRoot(String workspaceRoot) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. workspaceRoot(String) mutates defaultWorkspaceSpec directly. If a caller first supplies a WorkspaceSpec via workspaceSpec(WorkspaceSpec) and then calls workspaceRoot(String), the caller's spec object is mutated. Conversely, if workspaceRoot(String) is called first and workspaceSpec(WorkspaceSpec) second, the root set by workspaceRoot is silently discarded. Preserve the configured root when replacing defaultWorkspaceSpec, or copy the spec before mutating it. |
||
| options.setWorkspaceRoot(workspaceRoot); | ||
| // Copy-on-write so the caller's WorkspaceSpec is never mutated and the two builders | ||
| // stay order-independent regardless of which is applied last. | ||
| WorkspaceSpec spec = defaultWorkspaceSpec.copy(); | ||
| spec.setRoot(workspaceRoot); | ||
| this.defaultWorkspaceSpec = spec; | ||
| return this; | ||
| } | ||
|
|
||
|
|
@@ -111,10 +115,17 @@ public AgentRunFilesystemSpec snapshotSpec(SandboxSnapshotSpec snapshotSpec) { | |
| } | ||
|
|
||
| public AgentRunFilesystemSpec workspaceSpec(WorkspaceSpec workspaceSpec) { | ||
| this.defaultWorkspaceSpec = workspaceSpec; | ||
| this.defaultWorkspaceSpec = | ||
| workspaceSpec != null ? workspaceSpec.copy() : createDefaultWorkspaceSpec(); | ||
| return this; | ||
| } | ||
|
|
||
| private static WorkspaceSpec createDefaultWorkspaceSpec() { | ||
| WorkspaceSpec spec = new WorkspaceSpec(); | ||
| spec.setRoot(AgentRunSandboxState.DEFAULT_WORKSPACE_ROOT); | ||
| return spec; | ||
| } | ||
|
|
||
| @Override | ||
| protected SandboxClient<?> createClient() { | ||
| return client != null ? client : options.createClient(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,7 +137,7 @@ protected InputStream doPersistWorkspace() throws Exception { | |
| // Persistence is handled by the NAS/OSS mount — nothing to archive. | ||
| return InputStream.nullInputStream(); | ||
| } | ||
| String root = arState.getWorkspaceRoot(); | ||
| String root = arState.getWorkspaceSpec().getRoot(); | ||
| String cmd = "tar -cf - -C " + shellSingleQuote(root) + " . | base64 -w0"; | ||
| AgentRunMcpChannel.ExecResult r = mcp.exec(cmd, null, TAR_TIMEOUT_SECONDS); | ||
| if (r.exitCode != 0) { | ||
|
|
@@ -152,7 +152,7 @@ protected InputStream doPersistWorkspace() throws Exception { | |
|
|
||
| @Override | ||
| protected void doHydrateWorkspace(InputStream archive) throws Exception { | ||
| String root = arState.getWorkspaceRoot(); | ||
| String root = arState.getWorkspaceSpec().getRoot(); | ||
| byte[] all = archive.readAllBytes(); | ||
| if (all.length == 0) { | ||
| return; | ||
|
|
@@ -189,7 +189,7 @@ protected void doHydrateWorkspace(InputStream archive) throws Exception { | |
|
|
||
| @Override | ||
| protected void doSetupWorkspace() throws Exception { | ||
| mcp.exec("mkdir -p " + shellSingleQuote(arState.getWorkspaceRoot()), null, 30); | ||
| mcp.exec("mkdir -p " + shellSingleQuote(arState.getWorkspaceSpec().getRoot()), null, 30); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -199,15 +199,15 @@ protected void doDestroyWorkspace() throws Exception { | |
| return; | ||
| } | ||
| try { | ||
| mcp.exec("rm -rf " + shellSingleQuote(arState.getWorkspaceRoot()), null, 30); | ||
| mcp.exec("rm -rf " + shellSingleQuote(arState.getWorkspaceSpec().getRoot()), null, 30); | ||
| } catch (Exception e) { | ||
| // best-effort | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected String getWorkspaceRoot() { | ||
| return arState.getWorkspaceRoot(); | ||
| public String getWorkspaceRoot() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Warning] Removing public |
||
| return arState.getWorkspaceSpec().getRoot(); | ||
| } | ||
|
|
||
| private void ensureSandbox() throws Exception { | ||
|
|
@@ -238,7 +238,7 @@ private static boolean isNotFound(Exception e) { | |
| } | ||
|
|
||
| private String relativeOrAbsoluteCwd() { | ||
| String root = arState.getWorkspaceRoot(); | ||
| String root = arState.getWorkspaceSpec().getRoot(); | ||
| return root != null && !root.isBlank() ? root : null; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| /* | ||
| * 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.extensions.sandbox.agentrun; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import io.agentscope.harness.agent.sandbox.WorkspaceSpec; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class AgentRunFilesystemSpecTest { | ||
|
|
||
| @Test | ||
| void defaultWorkspaceSpec_hasAgentRunRoot() { | ||
| AgentRunFilesystemSpec spec = new AgentRunFilesystemSpec(); | ||
| WorkspaceSpec ws = spec.workspaceSpec(); | ||
| assertEquals(AgentRunSandboxState.DEFAULT_WORKSPACE_ROOT, ws.getRoot()); | ||
| } | ||
|
|
||
| @Test | ||
| void workspaceSpec_doesNotMutateCallerObject() { | ||
| WorkspaceSpec caller = new WorkspaceSpec(); | ||
| caller.setRoot("/caller/root"); | ||
|
|
||
| new AgentRunFilesystemSpec().workspaceRoot("/explicit/root").workspaceSpec(caller); | ||
|
|
||
| assertEquals("/caller/root", caller.getRoot()); | ||
| } | ||
|
|
||
| @Test | ||
| void workspaceRootAndWorkspaceSpec_areOrderIndependent() { | ||
| WorkspaceSpec caller = new WorkspaceSpec(); | ||
| caller.setRoot("/caller/root"); | ||
|
|
||
| // workspaceRoot applied last wins | ||
| WorkspaceSpec specLast = | ||
| new AgentRunFilesystemSpec() | ||
| .workspaceSpec(caller) | ||
| .workspaceRoot("/last") | ||
| .workspaceSpec(); | ||
| // workspaceSpec applied last wins (its root) | ||
| WorkspaceSpec specFirst = | ||
| new AgentRunFilesystemSpec() | ||
| .workspaceRoot("/last") | ||
| .workspaceSpec(caller) | ||
| .workspaceSpec(); | ||
|
|
||
| assertEquals("/last", specLast.getRoot()); | ||
| assertEquals("/caller/root", specFirst.getRoot()); | ||
| } | ||
|
|
||
| /** | ||
| * The mount check inside {@code AgentRunSandboxClient.create()} must run against the | ||
| * backend-defaulted root, not the raw spec: a NAS mount at the default AgentRun root has to | ||
| * be recognised so {@code doDestroyWorkspace()} does not rm -rf persistent data. | ||
| */ | ||
| @Test | ||
| void defaultRoot_isUnderAgentRunNasMount() { | ||
| String nasMount = AgentRunSandboxState.DEFAULT_WORKSPACE_ROOT; | ||
| WorkspaceSpec defaulted = | ||
| WorkspaceSpec.withDefaultRoot(null, AgentRunSandboxState.DEFAULT_WORKSPACE_ROOT); | ||
|
|
||
| assertEquals(nasMount, defaulted.getRoot()); | ||
| assertTrue(defaulted.getRoot().startsWith(nasMount)); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Info] Minor nit: |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ public class DaytonaFilesystemSpec extends SandboxFilesystemSpec { | |
| private SandboxClient<?> client; | ||
| private final DaytonaSandboxClientOptions options = new DaytonaSandboxClientOptions(); | ||
| private SandboxSnapshotSpec snapshotSpec = new NoopSnapshotSpec(); | ||
| private WorkspaceSpec defaultWorkspaceSpec = new WorkspaceSpec(); | ||
| private WorkspaceSpec defaultWorkspaceSpec = createDefaultWorkspaceSpec(); | ||
|
|
||
| public DaytonaFilesystemSpec client(SandboxClient<?> client) { | ||
| this.client = client; | ||
|
|
@@ -76,7 +76,11 @@ public DaytonaFilesystemSpec disk(int diskGiB) { | |
| } | ||
|
|
||
| public DaytonaFilesystemSpec workspaceRoot(String workspaceRoot) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. workspaceRoot(String) mutates defaultWorkspaceSpec directly. If a caller first supplies a WorkspaceSpec via workspaceSpec(WorkspaceSpec) and then calls workspaceRoot(String), the caller's spec object is mutated. Conversely, if workspaceRoot(String) is called first and workspaceSpec(WorkspaceSpec) second, the root set by workspaceRoot is silently discarded. Preserve the configured root when replacing defaultWorkspaceSpec, or copy the spec before mutating it.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. workspaceRoot(String) mutates defaultWorkspaceSpec directly. If a caller first supplies a WorkspaceSpec via workspaceSpec(WorkspaceSpec) and then calls workspaceRoot(String), the caller's spec object is mutated. Conversely, if workspaceRoot(String) is called first and workspaceSpec(WorkspaceSpec) second, the root set by workspaceRoot is silently discarded. Preserve the configured root when replacing defaultWorkspaceSpec, or copy the spec before mutating it. |
||
| options.setWorkspaceRoot(workspaceRoot); | ||
| // Copy-on-write so the caller's WorkspaceSpec is never mutated and the two builders | ||
| // stay order-independent regardless of which is applied last. | ||
| WorkspaceSpec spec = defaultWorkspaceSpec.copy(); | ||
| spec.setRoot(workspaceRoot); | ||
| this.defaultWorkspaceSpec = spec; | ||
| return this; | ||
| } | ||
|
|
||
|
|
@@ -86,10 +90,17 @@ public DaytonaFilesystemSpec snapshotSpec(SandboxSnapshotSpec snapshotSpec) { | |
| } | ||
|
|
||
| public DaytonaFilesystemSpec workspaceSpec(WorkspaceSpec workspaceSpec) { | ||
| this.defaultWorkspaceSpec = workspaceSpec; | ||
| this.defaultWorkspaceSpec = | ||
| workspaceSpec != null ? workspaceSpec.copy() : createDefaultWorkspaceSpec(); | ||
| return this; | ||
| } | ||
|
|
||
| private static WorkspaceSpec createDefaultWorkspaceSpec() { | ||
| WorkspaceSpec spec = new WorkspaceSpec(); | ||
| spec.setRoot(DaytonaSandboxState.DEFAULT_WORKSPACE_ROOT); | ||
| return spec; | ||
| } | ||
|
|
||
| @Override | ||
| protected SandboxClient<?> createClient() { | ||
| return client != null ? client : options.createClient(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workspaceRoot(String) mutates defaultWorkspaceSpec directly. If a caller first supplies a WorkspaceSpec via workspaceSpec(WorkspaceSpec) and then calls workspaceRoot(String), the caller's spec object is mutated. Conversely, if workspaceRoot(String) is called first and workspaceSpec(WorkspaceSpec) second, the root set by workspaceRoot is silently discarded. Preserve the configured root when replacing defaultWorkspaceSpec, or copy the spec before mutating it.