fix: use fs-safe trash for agents delete

[ferminquant]

Summary

• Problem: the offline CLI agents delete cleanup path shells out to a PATH-resolved trash binary.
• Why it matters: the official Docker runtime image does not include trash-cli, so deleting an agent can leave owned workspace/session paths behind after config state says the agent is gone.
• What changed: moveToTrash() now reuses OpenClaw's existing filesystem-safe movePathToTrash() helper with explicit allowed roots, and focused tests assert the helper route plus shared-workspace retention.
• What did NOT change (scope boundary): this does not add a Docker dependency, does not hard-delete workspaces, does not change JSON output shape, and does not change shared-workspace retention policy.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra
• Core CLI / agents

Linked Issue/PR

• Closes trash-cli missing from official Docker image, causing 'openclaw agents delete' to leave residuals #83459
• Related fix(onboard): fall back to fs.rm when 'trash' command is unavailable (#83459) #83472
• This PR fixes a bug or regression

AI assistance

This PR was implemented with Codex assistance. I reviewed the change and understand the affected agents delete cleanup path.

Real behavior proof (required for external PRs)

• Behavior or issue addressed: agents delete no longer depends on a PATH-resolved trash binary when moving owned agent paths to trash.
• Real environment tested: Locally built OpenClaw Docker runtime image from this branch. Inside the container, command -v trash was absent.
• Exact steps or command run after this patch: Built the repo Docker image, then ran openclaw agents add bug_test --workspace /tmp/openclaw-workspace --non-interactive --json followed by openclaw agents delete bug_test --force --json inside the container.
• Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):

Docker proof output

image: openclaw-83459-agents-delete-trash-helper:20260519T174205Z
timestamp_utc: 20260519T175450Z
container user: uid=1000(node) gid=1000(node) groups=1000(node)
trash binary absent as expected
{
  "agentId": "bug_test",
  "name": "bug_test",
  "workspace": "/tmp/openclaw-workspace",
  "agentDir": "/tmp/openclaw-state/agents/bug_test/agent",
  "bindings": {
    "added": [],
    "updated": [],
    "skipped": [],
    "conflicts": []
  }
}
workspace exists after add: yes
{
  "agentId": "bug_test",
  "workspace": "/tmp/openclaw-workspace",
  "agentDir": "/tmp/openclaw-state/agents/bug_test/agent",
  "sessionsDir": "/tmp/openclaw-state/agents/bug_test/sessions",
  "removedBindings": 0,
  "removedAllow": 0
}
PASS: workspace removed
PASS: workspace moved to trash: /tmp/openclaw-home/.Trash/openclaw-workspace-1779213303448-6JadRW/openclaw-workspace
END: explicit success
PROOF_STATUS=0
sg_status=0

• Observed result after fix: The workspace was removed from /tmp/openclaw-workspace and moved under the container user's ~/.Trash even though no trash binary was available.
• What was not tested: Cross-OS desktop Trash integration was not tested; this PR only changes the OpenClaw helper path used by the offline CLI agents delete cleanup path.
• Before evidence (optional but encouraged): Source-level proof on current main shows moveToTrash() invokes runCommandWithTimeout(["trash", pathname]), while the Docker runtime package list does not install trash-cli.

Root Cause (if applicable)

• Root cause: the offline CLI cleanup helper depended on an external trash executable instead of the existing in-process fs-safe trash helper.
• Missing detection / guardrail: command tests asserted the old PATH command behavior and did not prove Docker/minimal-Linux cleanup when trash was absent.
• Contributing context (if known): the Gateway delete path already used the safer movePathToTrash() style, so CLI and Gateway cleanup behavior diverged.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: src/commands/onboard-helpers.test.ts and src/commands/agents.delete.test.ts.
• Scenario the test should lock in: moveToTrash() calls movePathToTrash() with explicit allowed roots instead of invoking PATH trash, unique workspaces are trashed, and shared workspaces are retained.
• Why this is the smallest reliable guardrail: it covers the changed command helper and the agents delete caller without requiring broad Docker E2E for every run.
• Existing test that already covers this (if any): shared workspace retention had existing coverage; this PR updates it to assert the fs-safe helper route.
• If no new test is added, why not: N/A.

User-visible / Behavior Changes

openclaw agents delete now removes owned cleanup targets in Docker/minimal environments where the external trash command is absent. The behavior remains move-to-trash rather than hard-delete.

Diagram (if applicable)

Before:
openclaw agents delete -> offline cleanup -> PATH trash command missing -> target can remain

After:
openclaw agents delete -> offline cleanup -> fs-safe movePathToTrash -> target moved under ~/.Trash

Security Impact (required)

• New permissions/capabilities? (Yes/No) No
• Secrets/tokens handling changed? (Yes/No) No
• New/changed network calls? (Yes/No) No
• Command/tool execution surface changed? (Yes/No) Yes
• Data access scope changed? (Yes/No) No
• If any Yes, explain risk + mitigation: this removes a PATH-resolved external trash command from cleanup and reuses the existing fs-safe helper with explicit allowed roots. It does not add a new command execution path.

Repro + Verification

Environment

• OS: WSL Ubuntu / local OpenClaw checkout
• Runtime/container: locally built OpenClaw Docker runtime image from this branch
• Model/provider: N/A
• Integration/channel (if any): N/A
• Relevant config (redacted): isolated temp OpenClaw home/state/config inside the container

Steps

1. Build the OpenClaw Docker runtime image from this branch.
2. Confirm command -v trash is absent inside the container.
3. Run openclaw agents add bug_test --workspace /tmp/openclaw-workspace --non-interactive --json.
4. Run openclaw agents delete bug_test --force --json.
5. Assert /tmp/openclaw-workspace no longer exists and a matching workspace entry exists under ~/.Trash.

Expected

• agents delete removes the owned workspace from its original path and moves it to Trash-style storage without requiring a trash binary.

Actual

• Docker proof output showed trash binary absent as expected, PASS: workspace removed, PASS: workspace moved to trash, PROOF_STATUS=0, and sg_status=0.

Evidence

Attach at least one:

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

• Verified scenarios: focused command tests; changed-lane checks; Codex review; local no-trash proof; Docker runtime proof with no trash binary.
• Edge cases checked: shared workspace retention, unique workspace cleanup, target symlink outside parent, symlinked parent canonicalization, broken-symlink lexical fallback behavior delegated to fs-safe.
• What you did not verify: cross-OS desktop Trash integration and broad full-suite local pnpm test.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

No bot review conversations exist yet for this PR.

Compatibility / Migration

• Backward compatible? (Yes/No) Yes
• Config/env changes? (Yes/No) No
• Migration needed? (Yes/No) No
• If yes, exact upgrade steps: N/A

Risks and Mitigations

• Risk: filesystem cleanup helpers are sensitive to symlink/canonicalization behavior.
  • Mitigation: this PR reuses the existing fs-safe trash helper and adds focused symlink/allowed-root tests for the changed call site.
• Risk: the PR does not address the adjacent orphan parent-directory cleanup suggestion from the issue comments.
  • Mitigation: intentionally kept out of scope so this remains a narrow fix for the Docker missing-trash failure path.

Validation

• OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree corepack pnpm test src/commands/onboard-helpers.test.ts src/commands/agents.delete.test.ts
• PATH=/tmp/openclaw-corepack-bin:$PATH OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree corepack pnpm check:changed
• codex review --base origin/main
• Local no-trash proof with isolated OpenClaw home/state/config
• Docker runtime proof with no trash binary available

[clawsweeper]

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR replaces the CLI moveToTrash PATH trash command with @openclaw/fs-safe movePathToTrash and updates agents delete/onboarding tests for owned, shared, and symlinked cleanup paths.

Reproducibility: yes. from source and supplied Docker proof, though I did not execute it locally in this read-only review. Current main calls a PATH-resolved trash command while the default Docker runtime does not install trash-cli, and the PR body shows the after-fix no-trash Docker flow succeeding.

PR rating
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Summary: Focused bug fix with strong supplied Docker proof, dependency-backed implementation, and no blocking findings.

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.

Real behavior proof
Sufficient (live_output): The PR body includes after-fix Docker runtime output showing no trash binary and successful agents add then agents delete moving the workspace under .Trash.

Risk before merge

• Cross-OS desktop Trash integration was not directly exercised by the supplied Docker proof; confidence comes from the published @openclaw/fs-safe contract, existing helper tests, and current gateway/browser use of the same helper style.

Maintainer options:

1. Decide the mitigation before merge
   Land this fs-safe helper route after maintainer review and CI, then close trash-cli missing from official Docker image, causing 'openclaw agents delete' to leave residuals #83459 as fixed.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No repair lane is needed; the remaining path is ordinary maintainer review, CI, and landing for this recent focused PR.

Security
Cleared: No concrete security or supply-chain regression found; the patch removes a PATH-resolved external command and reuses the existing fs-safe helper with allowed roots.

Review details

Best possible solution:

Land this fs-safe helper route after maintainer review and CI, then close #83459 as fixed.

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

Yes from source and supplied Docker proof, though I did not execute it locally in this read-only review. Current main calls a PATH-resolved trash command while the default Docker runtime does not install trash-cli, and the PR body shows the after-fix no-trash Docker flow succeeding.

Is this the best way to solve the issue?

Yes. Reusing movePathToTrash with explicit allowed roots is narrower than adding a Docker dependency or hard-delete fallback, preserves recoverable cleanup, and aligns the offline CLI with the existing gateway cleanup helper.

What I checked:

• Current main shells out to PATH trash: moveToTrash on current main checks access and then calls runCommandWithTimeout(["trash", pathname], { timeoutMs: 5000 }), so a missing trash executable leaves cleanup as a logged best-effort failure. (src/commands/onboard-helpers.ts:248, a002c416c7af)
• Offline agents delete uses that helper for owned cleanup: The offline agents delete path prunes config/session state, retains shared workspaces, and calls moveToTrash for the unique workspace, agent dir, and sessions dir. (src/commands/agents.commands.delete.ts:150, a002c416c7af)
• Default Docker runtime does not install trash-cli: The runtime image installs a fixed base package list and optional extra apt packages via OPENCLAW_IMAGE_APT_PACKAGES; trash-cli is not in the default package list. (Dockerfile:163, a002c416c7af)
• PR switches the shared helper to fs-safe trash: The diff removes the runCommandWithTimeout(["trash", ...]) call and routes through movePathToTrash(sourcePath, { allowedRoots }), with tests asserting no PATH command is invoked. (src/commands/onboard-helpers.ts:202, a37d36c75f43)
• Dependency contract supports the helper route: @openclaw/fs-safe@0.2.4 exports movePathToTrash(targetPath, options) with allowedRoots; its implementation validates allowed roots, reserves a ~/.Trash destination, and avoids external PATH helpers.
• Gateway already uses fs-safe trash for agent deletion: The gateway agents.delete cleanup helper already calls movePathToTrash best-effort, so the PR aligns the offline CLI path with an existing runtime path. (src/gateway/server-methods/agents.ts:306, a002c416c7af)

Likely related people:

• Peter Steinberger: Introduced the onboarding moveToTrash helper in 7c2c541729, has many commits in the touched command/onboarding/fs-safe paths, and authored recent config/CLI refactors that shape this path. (role: feature owner and recent adjacent area contributor; confidence: high; commits: 7c2c54172931, 47216702f4f7, 4ee41cc6f31e; files: src/commands/onboard-helpers.ts, src/commands/onboard-helpers.test.ts, src/commands/agents.delete.test.ts)
• Advait Paliwal: Introduced the gateway agents.create/update/delete RPC surface in 980f788731, including the adjacent cleanup path that already uses fs-safe trash. (role: introduced related gateway agent deletion behavior; confidence: medium; commits: 980f788731f7; files: src/gateway/server-methods/agents.ts)
• Said Urtabajev: Authored the Docker build-arg policy commit that keeps extra apt packages optional, which affects whether trash-cli belongs in the default runtime image. (role: adjacent Docker image policy contributor; confidence: low; commits: 47b8e56e3f7b; files: Dockerfile, scripts/docker/setup.sh, docs/install/docker.md)
• Thomas Krohnfuß: Current-main blame for the cleanup helper and adjacent tests points at b7ba7c3f2a, so this is a useful recent-history routing signal even though older feature history is shared. (role: recent area contributor; confidence: medium; commits: b7ba7c3f2a1e; files: src/commands/onboard-helpers.ts, src/commands/agents.commands.delete.ts, src/plugin-sdk/browser-maintenance.test.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against a002c416c7af.

[clawsweeper]

ClawSweeper PR egg

✨ Hatched: 🥚 common Neon Review Wisp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

• Merged PRs are hatchable.
• Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
• Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: sniffs out flaky tests.
Image traits: location merge queue dock; accessory miniature diff map; palette pearl, teal, and neon green; mood watchful; pose nestled inside a glowing shell; shell translucent glimmer shell; lighting tiny status-light glow; background subtle branch markers.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Neon Review Wisp in ClawSweeper.

What is this egg doing here?

• Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
• The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
• Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
• The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
• Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

[RomneyDa]

lgtm!