fix(agent-core): keep Bash cwd applied to commands containing & - #2957
fix(agent-core): keep Bash cwd applied to commands containing ஍pucedoteth wants to merge 1 commit into
஍Conversation
The shell invocation was built as `cd <cwd> && <command>`. In POSIX shell
grammar `&&` binds tighter than `&`, so the cd is swallowed into the
command's first AND-list:
cd /d && a & b parses as { cd /d && a } & b
`{ cd /d && a }` runs in a background subshell, leaving the parent
shell's directory unchanged, so `b` runs wherever the session already
was rather than in the requested cwd. Verified against bash:
$ bash -c "cd '/tmp/cwdtest/target' && pwd & pwd"
/tmp/cwdtest <- wrong
/tmp/cwdtest/target
Terminating the cd on its own line keeps it a top-level command, so the
requested directory applies to the whole command:
$ bash -c "cd '/tmp/cwdtest/target' || exit 1
pwd & pwd"
/tmp/cwdtest/target
/tmp/cwdtest/target
Behaviour is otherwise unchanged: a failing cd still writes the same
message to stderr and exits 1, and `;`, trailing `&`, heredocs and
comment-only commands all behave as before.
Both engines built the string the same way, so both are fixed. The three
acp-server references to the old shape are doc comments only — the
matching logic there keys off `args[1] === '-c'` and
`shellCommand.endsWith(args.command)`, both of which still hold.
🦋 Changeset detectedLatest commit: 02a61eb The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![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: 02a61eb3d0
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `cd … && <command>` binds the cd into the command's first AND-list, so a | ||
| // command containing `&` runs everything after it in the shell's original | ||
| // directory (`cd /d && a & b` parses as `{ cd /d && a } & b`). Terminating | ||
| // the cd on its own line keeps the requested cwd applied to the whole | ||
| // command; a failing cd still prints to stderr and exits non-zero. |
There was a problem hiding this comment.
In this v2 file, the nearest AGENTS guidance requires comments to live only in the top-of-file block and never beside statements. This newly added statement comment therefore violates the local convention; please either fold the rationale into the existing header or remove it, keeping the executable line self-contained.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L36-L38
Useful? React with 👍 / 👎.
1 participant
Closes #2890.
Problem
Both Bash tools build the shell invocation by string concatenation:
`cd ${shellQuote(shellCwd)} && ${command}`In POSIX shell grammar
&&binds tighter than&, so thecdgets swallowed into the command's first AND-list:{ cd /d && a }runs in a background subshell, so the parent shell's directory never changes andbruns wherever the session already was — not in the requestedcwd. Verified against bash:;and a trailing&are unaffected —cdmutates the shell's own directory, and a whole-command background job still contains thecd. It is specifically an&between segments that loses it.Fix
Terminate the
cdon its own line so it stays a top-level command and the requested directory applies to everything that follows:`cd ${shellQuote(shellCwd)} || exit 1\n${command}`Everything else is byte-identical in behaviour. I checked each case against a real shell:
a & bbin wrong dircdbash: cd: …: No such file or directory, exit 1&a ; bScope
Both engines built the string identically, so both are fixed —
agent-core(v1) andagent-core-v2.acp-serverreferences the old shape in three doc comments, which I updated. Its actual matching logic is unaffected:isBashToolInvocationkeys offargs.length === 3 && args[1] === '-c'plus the noninteractive env, and terminal correlation usesevent.shellCommand.endsWith(command)— the invocation still ends with the model's command under the new form.I considered passing
cwdthroughProcessExecOptions(the runner already supports it) instead of a shellcd. I did not, because it moves the failure mode for a nonexistent directory from a shell-level message + exit 1 to a spawn-levelENOENT, and needs the native Windows path rather than thewindowsPathToPosixPathform used inside the shell. That felt like a bigger change than this bug warrants — happy to switch if you'd prefer it.Tests
Added
keeps cwd applied to commands containing a background operatorto the v2 suite. It fails onmain:13 existing assertions pinned the old literal string across three test files; those are updated to the new form.
vitest runon the three bash/shell test files: 133 passedoxlint --type-awareon the five touched files: 0 warnings, 0 errorsvitest run:1110 passed | 1 failed— the failure iskap-server test/prompts.test.ts > compresses inline base64 image prompts into session media-originals, which is unrelated to this change and flaky under full-suite parallelism: it passes in isolation both with my change and on a clean checkout (28/28 both ways), and a second full run failed a different set of tests.Changeset included (
patchon@moonshot-ai/kimi-codeand@moonshot-ai/kimi-code-sdk— both bundle the affected internal packages).