fix(agents): expand leading tilde in exec workdir#94450
Conversation
Add `expandTilde()` helper to handle `~` and `~/` prefixes in the `workdir` parameter of exec tool calls. The function expands leading tildes to the user's home directory before validating the path. Fixes issue openclaw#94434 where models emitting `workdir: "~/path"` would fail because the tilde was not expanded, causing the path validation to fail and fall back to a different directory while still executing the command. Changes: - Add `expandTilde()` function that handles `~`, `~/path`, and leaves other paths unchanged (absolute, relative, `~user` unsupported) - Modify `resolveWorkdir()` to call `expandTilde()` before validation - Add comprehensive unit tests for both functions covering all edge cases The fix is minimal and focused: only paths starting with `~` or `~/` are affected; all other path types continue to work as before. Fixes openclaw#94434
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close: this PR is superseded by the stronger open candidate at #94449, which targets the same exec workdir bug with OS-home semantics, focused coverage, and sufficient real behavior proof; this branch still has a changed-test import failure and only unit-test proof. Canonical path: Close this branch and continue the remaining semantic choice on #94449 for expansion behavior, or #94441 if maintainers choose fail-fast behavior. So I’m closing this here and keeping the remaining discussion on #94449 and #94441. Review detailsBest possible solution: Close this branch and continue the remaining semantic choice on #94449 for expansion behavior, or #94441 if maintainers choose fail-fast behavior. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows local gateway exec reaches Is this the best way to solve the issue? No. Expanding at the local workdir resolver is plausible, but this branch is not the best landing path because #94449 already covers the same expansion direction with cleaner OS-home semantics, proof, and focused tests. Security review: Security review cleared: No concrete supply-chain, permissions, secrets, or security-boundary regression was found in this small local exec workdir/test change; the blockers are duplicate landing path selection, proof, compatibility, and validation. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 269e44e16463. |
Summary
Fixes issue #94434 where the
exectool does not expand a leading tilde (~) in theworkdirparameter, causing commands to execute from an unintended fallback directory.The fix adds an
expandTilde()helper function that expands~and~/pathto the user's home directory before validating the workdir path. This allows models to safely use tilde notation in workdir paths.Fixes #94434
Real behavior proof
Behavior addressed: Exec tool calls with
workdir: "~/path"were failing because the tilde was not expanded, causing path validation to fail and fall back to a different directory while still attempting to execute relative commands.Real setup tested:
src/agents/bash-tools.shared.test.tsExact steps or command run after fix:
pnpm test src/agents/bash-tools.shared.test.tsAfter-fix evidence:
Observed result after the fix: All new unit tests pass, including:
expandTilde("~")returns home directoryexpandTilde("~/Documents")returns${homedir}/DocumentsresolveWorkdir("~/existing_dir", warnings)expands and validates successfullyresolveWorkdir("~/nonexistent", warnings)falls back with appropriate warningWhat was not tested: Live agent runs with actual model-generated tool calls (requires integration/E2E setup)
Tests and validation
Added 14 new unit tests covering all edge cases of tilde expansion and workdir resolution. All existing tests continue to pass.
Risk checklist
Did user-visible behavior change?
No- Only fixes previously broken behaviorDid config, environment, or migration behavior change?
NoDid security, auth, secrets, network, or tool execution behavior change?
NoWhat is the highest-risk area?
~or~/, which were previously brokenHow is that risk mitigated?
resolveWorkdir()and addsexpandTilde()helperCurrent review state
What is the next action?