fix(exec): fail invalid explicit workdir before running by renaudcerrato · Pull Request #94441 · openclaw/openclaw

renaudcerrato · 2026-06-18T05:26:27Z

Summary

Return a failed exec result when an explicit local workdir is unavailable instead of falling back to another directory.
Keep fallback behavior for implicit/default workdirs, and add a regression test that verifies the command is not run.

Real behavior proof

Behavior or issue addressed: Explicit invalid local exec.workdir no longer falls back to another directory and no longer executes the command from the wrong location.
Real environment tested: Local OpenClaw checkout at PR head 60a5c0456baea74b07a8852abb8fccd4c1ad6c50, Linux 6.17.0-121035-tuxedo, Node v22.22.1, host=gateway, security=full, ask=off.
Exact steps or command run after this patch: Ran a live node --import tsx --input-type=module script that imported the actual createExecTool from src/agents/bash-tools.exec.ts, called tool.execute(...) with an explicit missing workdir, and used a command that would write a marker file if it executed.
Evidence after fix: Live terminal output from the patched checkout:

{
  "timestamp": "2026-06-18T06:47:19.941Z",
  "commit": "60a5c0456baea74b07a8852abb8fccd4c1ad6c50",
  "openclawVersion": "2026.6.2",
  "platform": "linux 6.17.0-121035-tuxedo",
  "nodeVersion": "v22.22.1",
  "host": "gateway",
  "requestedWorkdir": "/tmp/openclaw-exec-proof-UJ3k3G/missing-workdir",
  "workdirExistsBeforeCall": false,
  "commandWouldCreateMarker": "/tmp/openclaw-exec-proof-UJ3k3G/should-not-exist.txt",
  "resultStatus": "failed",
  "resultExitCode": null,
  "markerExistsAfterCall": false,
  "toolText": "workdir \"/tmp/openclaw-exec-proof-UJ3k3G/missing-workdir\" not found: command was not executed. workdir is treated as a literal path; shell expansions such as \"~\" are not applied. Use an existing path or omit workdir."
}

Observed result after fix: The exec tool returned status=failed, reported workdir "..." not found: command was not executed, and markerExistsAfterCall=false, proving the command did not run from any fallback directory.
What was not tested: No additional gaps.

Tests

corepack pnpm exec vitest run --config test/vitest/vitest.agents.config.ts src/agents/bash-tools.exec.path.test.ts
corepack pnpm exec oxfmt --check --threads=1 src/agents/bash-tools.shared.ts src/agents/bash-tools.exec.ts src/agents/bash-tools.exec.path.test.ts
corepack pnpm tsgo:core && corepack pnpm tsgo:test:src

NianJiuZst · 2026-06-18T05:50:56Z

Thanks for the clean fix — the bug is well understood, the layer is right, and the explicit-vs-default workdir distinction matches exactly what the issue asks for.

Approve. A few non-blocking suggestions if you want a polish pass before merge; otherwise ship it.

What works:

The fix targets the local-exec preflight where the bug actually lives. Sandbox (resolveSandboxWorkdir) and node-host paths have their own resolution and are correctly left alone.
The default-workdir fallback is preserved for defaultWorkdir and process.cwd() cases — changing that would be a behavior break for users who never set workdir explicitly, and the issue explicitly only asked to fix the explicit case.
Extracting resolveExistingWorkdir (single responsibility, no side effects) and formatUnavailableWorkdirFailure (message lives in one place) is the right refactor. resolveWorkdir now composes them.
The regression test is targeted: os.tmpdir() + pid + Date.now() to avoid collisions, rmSync(force: true) to guarantee missing, and the strong assertion is result.details?.status === "failed". The user-visible message is pinned too.
Real behavior proof is exactly the right shape for this kind of bug — ran the actual createExecTool with a marker-file command and observed markerExistsAfterCall=false. Conclusive.
getWarningText hoisted before the early return so pre-existing warnings (e.g. approval warnings) still surface on the failure path. Good catch.

Minor suggestions (non-blocking):

The third assertion in the new test is weaker than it looks:
```
expect(value).not.toBe("ok");
```
The command is echo ok, which produces "ok\n" (with a trailing newline). If the command ever ran from a fallback dir again, value would be "ok\n", and not.toBe("ok") would still pass. The strong assertion above it (status: "failed") is what's actually catching the regression. Either drop this line or tighten it to expect(value).not.toMatch(/^ok/) so this line is a real second guard.
Optional: a one-line comment at the early return explaining why workdir check sits before rejectUnsafeExecControlShellCommand:
```
// Fail before any command-side validation: workdir availability is a
// precondition for execution.
```
Right now the rationale lives only in the PR description. A future reader of bash-tools.exec.ts will wonder about the ordering.
Optional follow-up (not for this PR): the inline buildExecForegroundResult({ outcome: { status: "failed", exitCode: null, exitSignal: null, durationMs: 0, aggregated: "", timedOut: false, failureKind: "runtime-error", reason } }) shape is verbose. If a third preflight failure path lands here (e.g. a future sandbox workdir check), a small buildPreflightFailureResult({ reason, warnings }) helper would centralize it. Two callers is below the bar; don't do it now, just flagging for the third caller.

Verdict: Approve. Nice, minimal, well-tested fix. Ship it.

clawsweeper · 2026-06-19T22:00:10Z

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 5:19 PM ET / 21:19 UTC.

Summary
The PR refactors exec workdir resolution, makes unavailable workdirs fail before execution, adds sandbox backend/OpenShell/SSH workdir validation APIs, and expands related tests and SDK baselines.

PR surface: Source +703, Tests +1955, Generated 0, Other +3. Total +2661 across 30 files.

Reproducibility: yes. Source inspection on current main shows local exec derives an explicit/default cwd, calls resolveWorkdir, stats the literal input, and falls back with a warning before command execution, matching the linked report.

Review metrics: 2 noteworthy metrics.

Plugin SDK sandbox surface: 3 types, 1 helper, 4 handle fields added. The PR expands a public plugin SDK subpath, so maintainers need to review compatibility and documentation before merge.
Configured/default cwd behavior: 2 default paths changed to fail-closed. Missing configured local and sandbox cwd defaults can break upgraded setups that previously warned and fell back.

Stored data model
Persistent data-model change detected: serialized state: src/agents/agent-tool-definition-adapter.test.ts, serialized state: src/agents/bash-tools.exec-workdir.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94434
Summary: This PR is a candidate fix for the canonical exec workdir leading-tilde/fallback bug, while another open sibling proposes tilde expansion instead of broader fail-fast behavior.

Members:

canonical: [Bug]: exec workdir with leading ~ falls back and still executes command #94434 - The PR body uses closing syntax for this open issue and implements one of the issue's stated acceptable outcomes.
partial_overlap: fix(exec): expand leading ~ in workdir to home directory #94449 - This open sibling targets the same report but chooses leading-tilde expansion while preserving fallback for nonexistent expanded paths.
superseded: fix: exec workdir with leading ~ falls back and still executes command #94447 - This closed unmerged sibling targeted the same local workdir bug with a smaller tilde-expansion approach.
superseded: fix(agents): expand leading tilde in exec workdir #94450 - This closed unmerged sibling targeted the same leading-tilde workdir behavior.
superseded: fix: expand leading tilde in exec workdir paths #94462 - This closed unmerged sibling targeted the same leading-tilde workdir behavior.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add current-head terminal or log proof for explicit local failure plus backend/OpenShell/SSH validation, with private details redacted.
[P1] Preserve default/configured cwd fallback unless maintainers explicitly accept fail-closed upgrade behavior.
Document the public sandbox backend SDK validation contract or keep the new seam internal.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal proof for an older local-only head, but current head is b08a768 and now covers backend/OpenShell/SSH validation, so current-head real behavior proof is still needed with private details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] Existing users with stale configured local or sandbox cwd values can move from warned fallback execution to fail-closed execution after upgrade.
[P1] The branch exposes a new public sandbox backend workdir validation lifecycle without public plugin SDK docs for third-party backend authors.
[P1] The supplied terminal proof predates current head and does not cover the widened backend/OpenShell/SSH validation paths.

Maintainer options:

Preserve explicit-only failure and document SDK validation (recommended)
Keep fail-fast behavior for explicit invalid workdirs, restore omitted/configured default fallback unless maintainers approve fail-closed behavior, add public SDK docs for backend validation, and require current-head proof.
Accept stricter defaults intentionally
Maintainers can keep fail-closed configured/default cwd behavior if the PR body, docs, and tests explicitly state the upgrade impact and prove the new behavior on current head.
Narrow back to local exec
If the backend SDK lifecycle is not ready, reduce this branch to the local explicit-workdir fix and leave sandbox backend validation for a separate design pass.

Next step before merge

[P1] Maintainer review is needed because the remaining blockers are compatibility/API direction and current-head proof, not a narrow automated repair lane.

Security
Cleared: No concrete security or supply-chain regression was found; the diff adds no dependencies or workflow permission changes, and the new remote validation command is built through the existing shell-escaping helper.

Review findings

[P1] Preserve fallback for omitted/default workdirs — src/agents/bash-tools.exec-workdir.ts:359-380
[P1] Document the sandbox backend workdir contract — src/plugin-sdk/sandbox.ts:17-33

Review details

Best possible solution:

Land one canonical exec workdir fix that fails explicit invalid workdirs before execution, preserves or explicitly approves upgrade behavior for defaults, and documents any public sandbox SDK validation seam.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows local exec derives an explicit/default cwd, calls resolveWorkdir, stats the literal input, and falls back with a warning before command execution, matching the linked report.

Is this the best way to solve the issue?

No, not yet. The explicit fail-fast direction is a strong candidate, but current head also changes omitted/configured defaults and exports a public sandbox SDK lifecycle without enough docs, upgrade proof, or current-head real behavior proof.

Full review comments:

[P1] Preserve fallback for omitted/default workdirs — src/agents/bash-tools.exec-workdir.ts:359-380
When no explicit workdir is supplied, this resolver now fails missing configured local and sandbox cwd values instead of falling back. Current main warns and falls back for these defaults, while the linked bug only requires fail-fast behavior for explicit invalid workdirs; this can break existing configs after upgrade unless maintainers deliberately accept it.
Confidence: 0.9
[P1] Document the sandbox backend workdir contract — src/plugin-sdk/sandbox.ts:17-33
These exports promote backend workdir validation into the public plugin SDK, but the PR does not add public docs for validateWorkdir, discardPreparedWorkdir, workdirRoots, timing, or compatibility expectations. Third-party sandbox backends would have to infer stable contract behavior from core internals.
Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 13ecca5408cb.

Label changes

Label justifications:

P2: This is a normal-priority exec runtime bug fix with bounded but real impact on agent commands that use workdirs.
merge-risk: 🚨 compatibility: The branch changes fallback/fail-closed behavior for configured defaults and expands public sandbox backend SDK semantics.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal proof for an older local-only head, but current head is b08a768 and now covers backend/OpenShell/SSH validation, so current-head real behavior proof is still needed with private details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +703, Tests +1955, Generated 0, Other +3. Total +2661 across 30 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	14	1055	352	+703
Tests	13	2071	116	+1955
Docs	0	0	0	0
Config	0	0	0	0
Generated	1	2	2	0
Other	2	5	2	+3
Total	30	3133	472	+2661

What I checked:

Repository policy applied: Root AGENTS.md requires full-policy review, flags fallback and plugin API changes as compatibility-sensitive, and requires whole-surface review before a PR verdict. (AGENTS.md:29, 13ecca5408cb)
Scoped plugin SDK policy applied: The plugin SDK guide identifies src/plugin-sdk as a public contract and requires docs, entrypoints, package exports, API baselines, and checks to stay aligned when a public subpath changes. (src/plugin-sdk/AGENTS.md:3, 13ecca5408cb)
Current main still has the reported fallback behavior: Current main's resolveWorkdir stats the raw workdir and falls back with a warning when unavailable, so the linked user problem is still real on main. (src/agents/bash-tools.shared.ts:183, 13ecca5408cb)
Current main caller reaches fallback before command execution: The local exec path derives explicit/default/current cwd and calls resolveWorkdir before continuing to command validation and execution. (src/agents/bash-tools.exec.ts:1570, 13ecca5408cb)
PR head changes configured/default cwd behavior: PR head returns unavailable for missing configured local and sandbox default cwd values when no explicit workdir was supplied, instead of preserving current fallback behavior. (src/agents/bash-tools.exec-workdir.ts:359, b08a768222b7)
PR tests pin fail-closed defaults: New tests expect missing configured local cwd and missing configured sandbox cwd to fail before launch, confirming this is intentional branch behavior rather than incidental implementation. (src/agents/bash-tools.exec-workdir.test.ts:133, b08a768222b7)

Likely related people:

steipete: The live PR is assigned to steipete, and git history shows Peter Steinberger commits touching exec preflight, cwd routing, and adjacent agent exec paths. (role: recent exec area contributor and assigned reviewer; confidence: high; commits: b71c91022b58, 247a06813e01, 39d1a817fa3f; files: src/agents/bash-tools.exec.ts, src/agents/bash-tools.shared.ts)
vincentkoc: Recent current-main history includes Vincent Koc commits splitting sandbox backend handle types and OpenShell backend types near the new SDK/backend validation surface; the sibling open PR also contains a vincentkoc commit for OS-home workdir semantics. (role: recent sandbox and plugin SDK area contributor; confidence: high; commits: 7d6af7e154c9, ce32697250d5, 710e0eff9d98; files: src/agents/sandbox/backend-handle.types.ts, extensions/openshell/src/backend.ts, src/agents/bash-tools.shared.ts)
jesse-merhi: The live PR timeline shows jesse-merhi assigned the review target and force-pushed the widened refactor/backend-validation commits now under review. (role: recent PR branch maintainer; confidence: medium; commits: b84d0a0e00be, 3440c068e09e, b08a768222b7; files: src/agents/bash-tools.exec-workdir.ts, extensions/openshell/src/backend.ts, src/plugin-sdk/sandbox.ts)
Starhappysh: Merged current-main history includes work on avoiding gateway cwd injection for node exec, which is adjacent to this PR's node/default cwd behavior. (role: adjacent exec cwd contributor; confidence: medium; commits: 247a06813e01; files: src/agents/bash-tools.exec.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

jesse-merhi · 2026-06-25T22:02:08Z

Merged via squash.

Prepared head SHA: b08a768222b7fe0d699c30866ca94f7b083aee75
Landed commit: 95b97e5b0b5e5c3ad00c7fbf1700973e22e50d8a

* fix(exec): fail invalid explicit workdir before running * test(exec): tighten invalid workdir regression * fix(exec): clarify invalid workdir recovery * refactor(exec): centralize workdir resolution * test(exec): update invalid workdir assertion * fix(exec): harden backend workdir contract * fix(exec): map missing backend host workdirs * fix(exec): reject control commands before workdir prep * fix(exec): defer env hook until backend cwd validation * chore(sdk): refresh plugin api baseline * test(agents): drop redundant definition assertions * test(exec): use real config workdirs * test(exec): use tracked temp dirs * test(openshell): keep temp setup local * test: update temp-dir route fixture --------- Co-authored-by: jesse-merhi <[email protected]>

This was referenced Jun 18, 2026

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

Closed

fix: exec workdir with leading ~ falls back and still executes command #94447

Closed

clawsweeper Bot mentioned this pull request Jun 19, 2026

fix: expand leading tilde in exec workdir paths #94462

Closed

clawsweeper Bot added 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

This was referenced Jun 19, 2026

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

Closed

fix(exec): expand leading ~ in workdir to home directory #94449

Closed

jesse-merhi assigned steipete Jun 25, 2026

renaudcerrato and others added 10 commits June 26, 2026 03:26

fix(exec): fail invalid explicit workdir before running

c910af3

test(exec): tighten invalid workdir regression

e65aa65

fix(exec): clarify invalid workdir recovery

b41511e

refactor(exec): centralize workdir resolution

b84d0a0

test(exec): update invalid workdir assertion

b494229

fix(exec): harden backend workdir contract

b050f3d

fix(exec): map missing backend host workdirs

4baa32b

fix(exec): reject control commands before workdir prep

5c26ec8

fix(exec): defer env hook until backend cwd validation

8d0b877

chore(sdk): refresh plugin api baseline

3440c06

jesse-merhi force-pushed the fix-exec-invalid-workdir branch from 04da69f to 3440c06 Compare June 25, 2026 17:49

jesse-merhi requested a review from a team as a code owner June 25, 2026 17:49

openclaw-barnacle Bot added docs Improvements or additions to documentation scripts Repository scripts docker Docker and sandbox tooling extensions: openshell size: XL and removed size: S labels Jun 25, 2026

jesse-merhi added 3 commits June 26, 2026 05:07

test(agents): drop redundant definition assertions

82d6013

test(exec): use real config workdirs

08339df

test(exec): use tracked temp dirs

8aba810

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 25, 2026

jesse-merhi added 2 commits June 26, 2026 06:52

test(openshell): keep temp setup local

85d23b4

test: update temp-dir route fixture

b08a768

jesse-merhi merged commit 95b97e5 into openclaw:main Jun 25, 2026
146 of 147 checks passed

jesse-merhi unassigned steipete Jun 26, 2026

Haderach-Ram mentioned this pull request Jun 26, 2026

Ecosystem Digest — 2026-06-26 Haderach-Ram/openclaw-radar#62

Open

clawsweeper Bot mentioned this pull request Jun 26, 2026

[Bug]: Agent self-update bypasses update.run and uses raw npm global upgrade without restarting gateway #64892

Open

jesse-merhi mentioned this pull request Jun 26, 2026

fix(exec): harden backend sandbox workdir audit fixes #96926

Merged

This was referenced Jun 26, 2026

feat: add tools.fs.roots — per-agent filesystem roots with access modes #52951

Closed

feat(mxc): add Windows MXC sandbox backend #97086

Merged

fix(node-host): clarify unusable node exec cwd failures #97105

Merged

clawsweeper Bot mentioned this pull request Jul 2, 2026

fix(sandbox): bound SSH command output buffering at 16 MiB #98829

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(exec): fail invalid explicit workdir before running#94441

fix(exec): fail invalid explicit workdir before running#94441
jesse-merhi merged 15 commits into
openclaw:mainfrom
renaudcerrato:fix-exec-invalid-workdir

renaudcerrato commented Jun 18, 2026 •

edited

Loading

Uh oh!

NianJiuZst commented Jun 18, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

jesse-merhi commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

renaudcerrato commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Real behavior proof

Tests

Uh oh!

NianJiuZst commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jesse-merhi commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

renaudcerrato commented Jun 18, 2026 •

edited

Loading

NianJiuZst commented Jun 18, 2026 •

edited

Loading

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading