fix(agent): detect shell completion marker on output without trailing newline#4744
fix(agent): detect shell completion marker on output without trailing newline#4744logicwu0 wants to merge 1 commit into
Conversation
logicwu0
changed the title
logicwu0
force-pushed
the
fix/4740-shell-marker-no-trailing-newline
branch
from
1de8219 to
1132247
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1132247329
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| int markerIndex = line.indexOf(marker); | ||
| if (markerIndex >= 0) { |
There was a problem hiding this comment.
When a user command reads from stdin (for example cat, head -n1, or read x; echo "$x"), it can consume and echo the injected marker command because the marker is written to the same shell stdin immediately after the user command. With the new indexOf check, the quoted marker inside that echoed command line is treated as completion even though the real marker command never ran, so exitCode stays null, the result is reported as successful, and long-running commands like cat can keep owning the session for subsequent executions. The previous startsWith behavior timed out and restarted in this case; this should only set completed after the suffix parses as the expected marker exit status.
Useful? React with 👍 / 👎.
|
@chatgpt-codex-connector 这个问题确实存在，感谢指出，已在 根因：marker 命令是紧接着用户命令写入同一个 stdin 的。当用户命令会读 stdin 时（如无参 修复：只有当 marker 之后的文本能解析成预期的退出码整数时，才置 |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c20a8cf2c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Integer parsedExitCode = parseExitCode(line.substring(markerIndex + marker.length())); | ||
| if (parsedExitCode != null) { |
There was a problem hiding this comment.
$LASTEXITCODE
When the default Windows shell (powershell.exe) runs a cmdlet or script that has not set $LASTEXITCODE, the marker emitted as Write-Output "<marker> $LASTEXITCODE" has no integer suffix; Microsoft documents $LASTEXITCODE as the exit code of the last native program or script and notes scripts can leave it unchanged (https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_automatic_variables). Because this branch now refuses to mark completion unless parseExitCode returns an integer, commands like Get-Location in a fresh PowerShell session are treated as normal output and wait until timeout/restart, whereas the previous code completed with exitCode == null.
Useful? React with 👍 / 👎.
logicwu0
force-pushed
the
fix/4740-shell-marker-no-trailing-newline
branch
from
7c20a8c to
0a81231
Compare
… newline ShellSessionManager.ShellSession#collectOutput detected the completion marker with startsWith, but when a command's output has no trailing newline (e.g. `cat` of a file without a final newline, or `printf` without `\n`) the line reader merges that trailing output and the marker onto one line, so the marker is never matched and collectOutput hangs until the command times out. Detect the marker anywhere in the line via indexOf, preserve any real output preceding it, and only treat it as completion when a valid exit code follows the marker. The latter guard is required because a stdin-reading command (e.g. no-arg `cat`) echoes the injected marker command itself before running; that echoed line contains the marker substring but no trailing exit code and must not falsely complete. Closes alibaba#4740
logicwu0
force-pushed
the
fix/4740-shell-marker-no-trailing-newline
branch
from
0a81231 to
970cf32
Compare
1 participant
What this PR does
Fixes #4740:
ShellSessionManagerhangs when a shell command's output has no trailing newline.Root cause
ShellSession#collectOutputdetects command completion by checking whether a stdout linestartsWiththe marker that is echoed after each command (echo <marker> $?). The stdout reader splits onBufferedReader.readLine(), so when the user command's output does not end with a newline (e.g.catof a file without a final newline, orprintf 'x'), that trailing output and the marker are read as a single line:startsWith(marker)is thenfalse, the marker is never matched, andoutputQueue.pollblocks until the command times out and the session is restarted — observed as a hang.Fix
Detect the marker anywhere in the line via
indexOf:The marker is
__LC_SHELL_DONE__+ a random UUID, so an in-line search won't false-match normal output.Tests
Added
ShellSessionManagerTest(no API key required):printf 'hello-no-newline'→ output captured, no timeout;printf 'oops' && false→ exit code still parsed from the merged line;Verified before the fix the first two time out (reproducing the hang); after the fix all pass.