Repository navigation
Conversation
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:
If the implementation already tracks that distinction, could you add an explicit regression test for the "only 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AgentScope-Java Version
2.1.0-SNAPSHOT, based on main
40a4a57ad79e82f8505572d3b195d0264ec7efcb.Description
Fixes #3645.
The sandbox workspace paragraph and file-tool path normalizer used
/workspaceeven when the active sandbox used a different directory. E2B also kept its execution root and workspace manifest root independent, including the default/home/userversus/workspacemismatch.Changes
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./home/useras the default; either root setter can configure the directory. Copy the caller's specification so entries/environment survive without mutating it./workspace.Compatibility decision for review
As asked in the issue comment, this implementation rejects explicitly conflicting
workspaceRoot(...)andworkspaceSpec(...).rootvalues 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
/workspacefor an active/home/usersandbox, and default/custom E2B roots did not reach the workspace specification. The two existing sandbox-prompt tests passed.Final command from the repository root:
read_filetool, prefix boundaries and released acquisitions.git diff --checkpassed.docs/:npm testpassed 13 tests;npm run checkpassed for 430 pages/584 redirects;npm run broken-linksreported no broken links.Remaining validation gaps: full-repository offline
clean verifycould not start becausemaven-clean-plugin:3.5.0was not cached.npm run validatepassed the local checker but the Mintlify build did not complete (initial network-check exception, then a network-enabled retry remained atvalidating buildand was interrupted). No full-suite or remote-sandbox success is claimed.Checklist
mvn spotless:applymvn test) — targeted results above; full suite not completed