Conversation
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
Well-diagnosed fix. Switching the embedded python3 -c program to a Java text block makes the line feeds real (a literal \\n inside POSIX double quotes reaches python3 unchanged and collapses the program into one line), and the added isSuccess() guard aligns edit() with the existing executeFailureMessage(...) convention used by ls/read/grep/glob. CLA signed; unit + Linux real-shell regression tests are a good addition. LGTM.
One note: this PR currently shows mergeStateStatus=BLOCKED on GitHub — likely pending CI approval/reviews, not a code issue.
Automated review by github-manager-bot
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
CLA Not Signed The Contributor License Agreement (CLA) check is currently pending on this PR ( @aiyili 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 Automated check by github-manager-bot |
|
@cla-assistant check |
… SyntaxError - 改用 Java 文本块构建 python3 -c 程序,语句间为真实换行符:POSIX 双引号字符串内 字面反斜杠+n 序列会原样传给 python3,整个程序被压成一行直接 SyntaxError - execute 返回后先检查退出码:执行层失败(超时/传输错误/python 崩溃)现在携带 原始原因返回,不再被掩盖成 unexpected server response - 新增单元测试(生成命令形态 + 失败响应)与 Linux 真实 shell 集成测试 (单次替换 / 多处出现拒绝 / replaceAll 全替换) Fixes agentscope-ai#2571
|
Cross-referencing the #2571 evaluation's two reviewer asks against this PR: 1. The extra 2. Pinning the command construction — Additionally, this PR also fixes the second symptom from the report — the exit code is checked before parsing, so execution-layer failures (timeout / transport / interpreter crash) surface with their cause instead of being masked as Verified end-to-end against a real |
Fixes #2571
Problem
BaseSandboxFilesystem.edit(...)failed before reading or modifying the target file. Two defects in the generated shell command:Literal
sequences instead of line feeds. The Java code joined thepython3 -c "..."program fragments with\n(a literal backslash + n at runtime). Inside a POSIX double-quoted string the backslash beforenis not an escape — the two characters reach python3 unchanged — so the whole program arrived as a single line and the interpreter failed with:Exit code ignored.
execute's response was parsed without checking the exit code, so the interpreter crash above was masked asunexpected server responseinstead of the actual cause.Fix
result.isSuccess()right afterexecute, reusing the existingexecuteFailureMessage(...)convention fromls/read/grep/glob: execution-layer failures (timeout 124, transport errors, interpreter crash) now surface with their diagnostic output (or exit code fallback) instead of being mislabeled.Verification
sh+ python3 locally: the old command fails with theSyntaxErrorabove and leaves the file untouched; the fixed command outputs{"count": 1}and performs the replacement.agentscope-harnessmodule test suite: BUILD SUCCESS;BaseSandboxFilesystemTest19/19 canned tests pass (incl. 3 new ones), 3 new Linux real-shell integration tests run on CI.mvn spotless:checkpasses.Tests added
Unit (all platforms):
edit_generatedPythonProgram_isSeparatedByRealLineFeeds— the generated command contains real line feeds and no literalsequenceedit_executeFailure_shouldFailWithCause— transport failure surfaces with its causeedit_executeFailure_nullOutput_shouldFailWithExitCodeFallback— exit-code fallback messageLinux real-shell integration:
edit_singleOccurrence_replacesIt—replaceAll=falseon a single occurrence replaces it (the scenario from the issue)edit_multipleOccurrencesWithoutReplaceAll_failsAndKeepsFileUnchangededit_replaceAll_replacesEveryOccurrence