fix(harness): build sandbox commands without load-bearing quotes

[King52HerTz]

What this fixes

BaseSandboxFilesystem.move() built

mkdir -p $(dirname '<path>') && mv '<from>' '<to>'

The command substitution is unquoted, so a destination whose parent directory contains a space word-splits into two arguments. Reproduced locally on macOS with a plain sh:

$ sh -c "mkdir -p \$(dirname '/tmp/exp/dst dir/c d.txt') && mv '/tmp/exp/srcdir/a b.txt' '/tmp/exp/dst dir/c d.txt'"
mv: rename /tmp/exp/srcdir/a b.txt to /tmp/exp/dst dir/c d.txt: No such file or directory

Two directories named dst and dir get created as a side effect, and the rename never happens. This is not Windows-specific — it fails on any POSIX host. The parent directory is now derived in Java and passed as one single-quoted argument.

Windows host transport (issue #3262)

write() wrapped $(dirname ...) in double quotes. A sandbox command travels as a single argv element, and on a Windows host that element is serialized for CreateProcess and re-parsed by docker.exe; the report in #3262 shows the inner double quotes gone while single-quoted text survives. write() and move() are now generated without any double quote - the parent directory is computed in Java (FilesystemUtils.mkdirParent) instead of by a shell dirname.

edit() is deliberately left as it is on main: #3226 already rewrote that command with a Java text block plus an exit-code check while this PR was open, and its approach is the one that should stay. Consequently the edit_file half of #3262 is still open - that command still nests double quotes around the Python program, so on a Windows host it can lose them the same way. I did not fold that into this PR, to keep the two changes reviewable separately; happy to take it as a follow-up.

To be explicit about what I verified and what I did not: the move() failure and the "no nested double quotes" property are covered by tests that run on Linux/macOS. I have no Windows host, so the Windows round trip itself is untested here - the change removes the construct #3262 identifies as the cause, it does not prove the outcome on Windows.

Also

A write rejected by the sandbox returned Failed to write file '<path>' with the command output discarded, which is why #3262 reads as a tool failing for no reason. It now goes through executeFailureMessage, matching how ls/read/grep/glob - and, since #3226, edit() - already report failures.

Testing

BaseSandboxFilesystemTest.CommandConstructionTests covers the spaced-parent move, the absence of nested double quotes in the two generated commands, the failure message, and parentDir edge cases (absolute / relative / root / bare / empty / null). Three separate mutations, each killed by exactly one designated test with no collateral failures:

reverted	failing test	reported
the move() line only	move_destinationParentWithSpace_renamesAndCreatesExactlyThatDirectory	mv: rename ... No such file or directory
write() back to "$(dirname ...)"	generatedCommands_avoidNestedDoubleQuotes	the offending command echoed in full
write()'s failure message	write_sandboxFailure_surfacesCommandOutput	Failed to write file '/workspace/note.txt'

Full reactor, same command as CI:

mvn -B -T1 clean verify
# 97/97 modules SUCCESS, 12:58 min, 0 failures / 0 errors, spotless:check included

Related, deliberately left out of this PR

• [Bug]:window系统下执行任何命令工具 execute 总是出错 #2271 and [Bug]:Command Execution Fails on Windows with "Cannot run program 'sh'" Error #1396 are the same area but a different defect: the generated commands are POSIX shell programs (if [ -e ... ], heredocs) that cmd.exe cannot execute at all, regardless of quoting. Fixing those needs a notion of which shell a backend runs programs in, which is a larger change. Not claimed here.
• shellSingleQuote is duplicated in five places (SandboxBackedFilesystem, DaytonaSandbox, E2bSandbox, AgentRunSandbox, SharedSandboxFilesystem) with two different escaping algorithms, and three of them emit '"'"' — which reintroduces double quotes and so carries the same Windows exposure. SharedSandboxFilesystem also has an unquoted mkdir -p $(dirname ...) like the one fixed here. Happy to follow up with a single shared helper if that is wanted.

Partially addresses #3262

[CLAassistant]

All committers have signed the CLA.

[oss-maintainer]

Summary

PR #3329 — fix(harness): build sandbox commands without load-bearing quotes. The motivation is well founded (nested double quotes do not survive a Windows host re-serialising the command line, per #3262) and the new CommandConstructionTests are a welcome addition. One regression to settle before merge: the base64 payload now travels as an environment assignment on the command line, which reintroduces an OS argument-length ceiling that the previous stdin heredoc did not have.

Findings

• [Warning] agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/sandbox/BaseSandboxFilesystem.java:262 — SAA_EDIT_PAYLOAD='<base64>' is a single argv string; Linux caps that at 128 KiB (MAX_ARG_STRLEN), so large edit() payloads fail with Argument list too long. Keep the payload on stdin or bound it explicitly.

Verified

• The added @EnabledOnOs({OS.LINUX, OS.MAC}) / @Test annotations are valid — EnabledOnOs and OS are already imported in BaseSandboxFilesystemTest.java, so no compile issue there.
• Back-compat looks fine for the FilesystemUtils addition (new overload, existing shellQuote callers unchanged).

CLA note

license/cla is pending on this head commit, so this is a comment-only review (no approval) — @King52HerTz please sign the CLA.

---

Automated review by github-manager-bot

[oss-maintainer]

[Warning] Moving the payload into a single environment-variable assignment puts it on the command line, where Linux's per-string execve limit (MAX_ARG_STRLEN, 128 KiB) applies — the previous heredoc-on-stdin form had no such bound. edit() calls whose base64 payload exceeds ~128 KiB (roughly 96 KiB of JSON content) will now fail with Argument list too long, which is a new failure mode for large edits. Suggest keeping the payload on stdin (and single-quoting / escaping the python3 - program) so the command stays quote-free without reintroducing a size ceiling; if an env-var carrier is intended, please cap the payload and return a clear error above the threshold.

[oss-maintainer]

CLA Not Signed

The Contributor License Agreement (CLA) check is currently pending on this PR (license/cla: Contributor License Agreement is not signed yet.). This PR cannot be merged until the CLA is signed.

@King52HerTz please sign the CLA via the CLA assistant badge in the comment above, or visit https://cla-assistant.io/agentscope-ai/agentscope-java. Once signed, the license/cla status will turn green.

---

Automated check by github-manager-bot

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[King52HerTz]

Following up on the first CI run, which failed LocalFilesystemPersonalAssistantExampleTest.localFilesystem_filesPersistAcrossCalls on both ubuntu and windows. The re-run triggered by the empty commit above turns ubuntu green with the same source, so I looked for a real coupling before changing anything. I could not find one, and I now believe the failure is a pre-existing timing race in that test rather than a consequence of this diff.

Why this diff is not on that test's path:

• The test writes MEMORY.md through WorkspaceManager.writeUtf8WorkspaceRelative, which calls filesystem.uploadFiles(...) (WorkspaceManager.java:723). This branch does not touch uploadFiles.
• The only methods changed here are BaseSandboxFilesystem.write(), .edit() and .move(). Within this module move() is reached from WorkspaceManager.moveSkill(), which promotes skill directories and has nothing to do with MEMORY.md.
• The observed symptom also fits a race, not a quoting change: the assertion compares MEMORY.md after a second agent.call(...), and the value it got (done, the stub model's output) is what memory consolidation writes. Whether that async work lands before the assertion is the test's own timing dependency; locally it never does.

What I ran locally, all green on this branch:

run	result
the failing class alone	3/3 pass
whole agentscope-harness module	509 pass, 0 failures
full reactor mvn test (all 96 modules)	pass
my new CommandConstructionTests + the example test in one JVM	both pass

The move() fix here is independently verifiable and does not depend on timing: the old mkdir -p $(dirname 'x/y z.txt') word-splits on any POSIX shell, creating stray y/z.txt directories and aborting the rename. I confirmed by reverting only that line — the new test then fails with mv: rename ... No such file or directory.

Happy to restructure if a maintainer would rather see this split, and fine with re-running the suite as-is. Could someone trigger a re-run of the harness job on the current head? I do not have permission to re-run it myself.

[oss-maintainer]

⚠️ Merge conflict detected

This PR currently conflicts with main and cannot be merged. Please rebase (or merge main into your branch) and resolve the conflicts:

git fetch origin
git checkout fix/sandbox-command-quoting
git rebase origin/main
# resolve conflicts, then:
git push --force-with-lease

This is a one-time reminder. Feel free to @mention me for a re-review once the conflict is resolved.

---

Automated notification by github-manager-bot

[oss-maintainer]

Status refresh (no action needed from you)

Re-checked the current state of this PR:

• CI on head 2863c0eb: all green — build (ubuntu-latest), build (windows-latest), Check License, Check Module Sync, codecov/patch all pass. Your analysis in the previous comment (the localFilesystem_filesPersistAcrossCalls failure being a pre-existing timing race, not caused by this diff) is consistent with what the re-run shows, so no further investigation is needed on your side.
• Still blocking:
  1. license/cla is pending — the CLA needs to be signed via the cla-assistant link in the checks list before this can be approved or merged.
  2. The branch has conflicts with main (CONFLICTING / DIRTY), see the earlier reminder comment.

Nothing else to do until those two are resolved — this is a status note, not a new request.

---

Automated notification by github-manager-bot

[King52HerTz]

The build (windows-latest) failure here doesn't look like it belongs to this change — it looks like a timing flake on the Windows runner.

• This run (38130992868, step Build and Test with Coverage [Windows]) failed 2 of 1228 harness tests: SubagentPendingToolRecoveryTest.rebuiltChildRespectsRecoveryAfterPersistedActingFailure[2] and [3]. The whole class took 44.43 s here.
• The assertion that fails is assertSame(failure, assertThrows(IllegalStateException.class, () -> child.call(...).block(TIMEOUT))): expected the injected acting failed before tool execution, got Timeout on blocking read for 10000000000 NANOSECONDS — i.e. the pipeline didn't settle inside the test's own TIMEOUT = Duration.ofSeconds(10), and Reactor's timeout exception is also an IllegalStateException, so it slips past assertThrows and only the identity check catches it.
• Same class, same runner, unrelated PRs, all green: 5.60 / 8.21 / 13.51 / 21.72 s (runs 38128099122, 38141427129, 38191148704, 38148893365). Those same four runs each went red on a different wall-clock-sensitive test instead: SharedWorkspaceSkillStagingE2ETest (87.20 s, 120.40 s), SubagentRunIdChainTest (1 error), AguiInputErrorTest (1 failure).
• On this head (0616683b9) locally: mvn -pl :agentscope-harness -am -Dtest=SubagentPendingToolRecoveryTest test → 23 run, 0 failures, 3.31 s, BUILD SUCCESS.
• This PR only changes BaseSandboxFilesystem.write()/move(), adds FilesystemUtils.parentDir()/mkdirParent(), and extends BaseSandboxFilesystemTest. The failing test goes through LocalFilesystem (implements AbstractFilesystem), not BaseSandboxFilesystem, and does not reference either changed class.

Could a maintainer re-run the failed Windows job, or is windows-latest already tracked as flaky here? If it's the latter, widening that 10 s block timeout (or making the assertion distinguish Reactor's timeout from the injected failure) would probably make these runs quieter — happy to do that as a separate change if you want it.