Skip to content

fix(harness): honor active sandbox workspace roots - #3646

Open
logicwu0 wants to merge 1 commit into
agentscope-ai:mainfrom
logicwu0:fix/3645-sandbox-workspace-root
Open

logicwu0 wants to merge 1 commit into
agentscope-ai:mainfrom
logicwu0:fix/3645-sandbox-workspace-root

Conversation

@logicwu0

Copy link
Copy Markdown
Contributor

AgentScope-Java Version

2.1.0-SNAPSHOT, based on main 40a4a57ad79e82f8505572d3b195d0264ec7efcb.

Description

Fixes #3645.

The sandbox workspace paragraph and file-tool path normalizer used /workspace even when the active sandbox used a different directory. E2B also kept its execution root and workspace manifest root independent, including the default /home/user versus /workspace mismatch.

Changes

  • Resolve the sandbox root from the acquisition bound to the current RuntimeContext, not a shared filesystem field. Use this for the workspace paragraph and registered filesystem/artifact path normalization. Restored state takes precedence over build-time defaults; released acquisitions are rejected.
  • Preserve nonmatching and relative paths for the existing filesystem policy. Match prefix boundaries and handle trailing slashes and the filesystem root.
  • Align E2B's effective workspace specification and execution options. Keep /home/user as the default; either root setter can configure the directory. Copy the caller's specification so entries/environment survive without mutating it.
  • When constructing an E2B sandbox from older state, retain its saved execution directory and align only the manifest. No files are moved, and no lifecycle or snapshot algorithm is changed.
  • Add regression tests and matching English/Chinese sandbox documentation. Docker's default remains /workspace.

Compatibility decision for review

As asked in the issue comment, this implementation rejects explicitly conflicting workspaceRoot(...) and workspaceSpec(...).root values when building the context, independent of setter order (trailing slashes are ignored). This policy is proposed for review, not yet maintainer-approved. Existing persisted E2B state is handled differently: its saved execution root stays authoritative so resuming does not relocate the workspace. Please confirm whether fail-fast is preferred for new conflicting configurations.

Validation

JDK 21, Java 17 compilation target, Maven 3.10.0. No real model or E2B service was contacted.

Before the fix, three new offline assertions failed: the prompt advertised /workspace for an active /home/user sandbox, and default/custom E2B roots did not reach the workspace specification. The two existing sandbox-prompt tests passed.

Final command from the repository root:

mvn -o -B -T1 -ntp \
  -pl agentscope-extensions/agentscope-extensions-sandbox/agentscope-extensions-sandbox-e2b -am \
  '-Dtest=SandboxWorkspaceRootTest,WorkspaceContextMiddleware*Test,HarnessAgentTest,*Sandbox*Test,FilesystemToolTest' \
  -Dsurefire.failIfNoSpecifiedTests=false spotless:apply test
  • Harness: 224 tests, 0 failures/errors, 17 skipped.
  • E2B: 57 tests, 0 failures/errors, 1 skipped.
  • Total: 263 passed, 18 skipped. The selection includes existing sandbox recovery, file-transfer and Harness tests; this is not a full-repository result.
  • Regression coverage includes default/custom roots, setter order/conflicts, invalid roots, legacy-state resume, caller-owned manifest preservation, per-call root isolation, the registered read_file tool, prefix boundaries and released acquisitions.
  • Spotless checks and git diff --check passed.
  • In docs/: npm test passed 13 tests; npm run check passed for 430 pages/584 redirects; npm run broken-links reported no broken links.

Remaining validation gaps: full-repository offline clean verify could not start because maven-clean-plugin:3.5.0 was not cached. npm run validate passed the local checker but the Mintlify build did not complete (initial network-check exception, then a network-enabled retry remained at validating build and was interrupted). No full-suite or remote-sandbox success is claimed.

Checklist

  • Code has been formatted with mvn spotless:apply
  • All tests are passing (mvn test) — targeted results above; full suite not completed
  • Javadoc comments are complete and follow project conventions
  • Related documentation has been updated
  • Code is ready for review, including the explicit configuration-conflict policy question above

@Peng-io

Peng-io commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking this up — the scope looks right.

On the compatibility question: fail-fast for explicit conflicts is what I'd prefer, with one distinction that I think is the crux of the original report:

  • workspaceRoot("/home/user") set on its own, with WorkspaceSpec.root left at its /workspace default, is not a conflict. It is exactly the configuration that produced this issue. It should be treated as "only the execution root was configured" and copied into the effective WorkspaceSpec.root, not rejected — otherwise every existing E2B caller that only sets workspaceRoot(...) starts failing at build time after upgrading.
  • Rejecting should apply only when the caller sets both explicitly and the values differ.

If the implementation already tracks that distinction, could you add an explicit regression test for the "only workspaceRoot(...) set, manifest left at its default" case? That is the scenario from the report and by far the most common configuration.

For persisted state, keeping the saved execution directory authoritative and only aligning the manifest is the right call. Relocating an existing workspace would be far more disruptive than the mismatch itself.

Thanks.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Sandbox system prompt and path normalizer hardcode "/workspace", ignoring the configured sandbox root

2 participants