Skip to content

fix(agents): expand leading tilde in exec workdir#94450

Closed
bowenluo718 wants to merge 1 commit into
openclaw:mainfrom
bowenluo718:fix/issue-94434-bug-exec-workdir-with-leadi
Closed

fix(agents): expand leading tilde in exec workdir#94450
bowenluo718 wants to merge 1 commit into
openclaw:mainfrom
bowenluo718:fix/issue-94434-bug-exec-workdir-with-leadi

Conversation

@bowenluo718

Copy link
Copy Markdown

Summary

Fixes issue #94434 where the exec tool does not expand a leading tilde (~) in the workdir parameter, causing commands to execute from an unintended fallback directory.

The fix adds an expandTilde() helper function that expands ~ and ~/path to 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:

  • Runtime: Node.js test environment with Vitest
  • Test file: src/agents/bash-tools.shared.test.ts

Exact steps or command run after fix:

pnpm test src/agents/bash-tools.shared.test.ts

After-fix evidence:

 RUN  v4.1.8 /home/.../issue-94434

 ✓ src/agents/bash-tools.shared.test.ts (25 tests) 240ms

 Test Files  1 passed (1)
      Tests  25 passed (25)
   Duration  1.38s

Observed result after the fix: All new unit tests pass, including:

  • expandTilde("~") returns home directory
  • expandTilde("~/Documents") returns ${homedir}/Documents
  • resolveWorkdir("~/existing_dir", warnings) expands and validates successfully
  • resolveWorkdir("~/nonexistent", warnings) falls back with appropriate warning

What 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 behavior

Did config, environment, or migration behavior change? No

Did security, auth, secrets, network, or tool execution behavior change? No

What is the highest-risk area?

  • Minimal risk: The change only affects paths starting with ~ or ~/, which were previously broken

How is that risk mitigated?

  • Comprehensive unit test coverage (14 new tests)
  • Narrow scope: only modifies resolveWorkdir() and adds expandTilde() helper

Current review state

What is the next action?

  • Maintainer review

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
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 18, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

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 details

Best 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 resolveWorkdir(), and current main plus v2026.6.8 still stat the literal ~/... path before falling back.

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:

  • Current main still has the bug path: Local gateway exec still passes explicit/default workdirs into resolveWorkdir(), and resolveWorkdir() still calls statSync(workdir) on the raw value before falling back. (src/agents/bash-tools.shared.ts:187, 269e44e16463)
  • Latest release is still affected: Release v2026.6.8 has the same raw statSync(workdir) fallback implementation, so the linked bug is real but not fixed by this stale branch being open. (src/agents/bash-tools.shared.ts:187, 844f405ac1be)
  • This branch has a concrete test import failure: The PR adds statSync from node:fs/promises, but that module does not export statSync; the PR CI check-test-types job reports TS2305 at the changed import line. (src/agents/bash-tools.shared.test.ts:7, 5a21c8c0dd60)
  • Runtime contract confirms the import problem: Node exposes statSync from node:fs, not from node:fs/promises, matching the CI type failure and the unused-import lint findings. (src/agents/bash-tools.shared.test.ts:7, 5a21c8c0dd60)
  • Canonical expansion candidate is stronger: Live PR data shows fix(exec): expand leading ~ in workdir to home directory #94449 is open, mergeable, proof-sufficient, and its ClawSweeper review identifies it as the clean OS-home expansion candidate for the same issue. (710e0eff9d98)
  • OS-home helper contract checked: Existing host path utilities distinguish OS-home expansion from OpenClaw effective-home expansion, and the canonical sibling covers the divergent HOME versus OPENCLAW_HOME behavior that this branch does not cover. (src/infra/home-dir.ts:65, 269e44e16463)

Likely related people:

  • vincentkoc: Authored the open canonical OS-home expansion commit for this exact resolver and current-line blame in this shallow checkout also routes the shared resolver area to Vincent Koc. (role: recent canonical fix owner; confidence: high; commits: 710e0eff9d98, 3283540c78ae; files: src/agents/bash-tools.shared.ts, src/agents/bash-tools.shared.test.ts)
  • steipete: GitHub commit metadata shows the agent tool split commit added both bash-tools.exec.ts and bash-tools.shared.ts, including the shared exec helper surface. (role: introduced behavior and exec area contributor; confidence: medium; commits: e2f89099829c; files: src/agents/bash-tools.exec.ts, src/agents/bash-tools.shared.ts)
  • openperf: Merged work on the same exec workdir routing surface changed remote-node cwd handling and is relevant to local-versus-node workdir semantics. (role: adjacent exec workdir contributor; confidence: medium; commits: 323bc9d8c764, 7d3f5f01e6d8, 116258b0ce03; files: src/agents/bash-tools.exec.ts)

Codex review notes: model internal, reasoning high; reviewed against 269e44e16463.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: exec workdir with leading ~ falls back and still executes command

1 participant