fix(root): report operational remove failure codes

[steipete]

Summary

This is the maintainer-finished continuation of #84. It preserves @Yigtwxx's two commits and credit, then fixes the remaining public error-category mismatch.

• report not-found, not-empty, and not-removable from the target deletion stage of Root.remove()
• keep ambiguous parent-directory guard failures fail-closed as path-alias
• categorize the three routine filesystem outcomes as operational, with regression coverage for both the exported categorizer and constructed FsSafeError instances
• document the compatibility change in the README, error reference, type reference, and changelog

Category decision

All three codes are operational. not-found is used package-wide for ordinary absence, not-empty is a routine rmdir result, and not-removable represents deletion syscall failures such as permissions, a busy device, or a read-only filesystem. None indicates unsafe input, path aliasing, or identity drift.

Because category is derived solely from code, this also changes pre-existing not-found errors outside remove() from policy to operational. That preserves the invariant that one exported code has one category. No unrelated category was changed: no-clobber outcomes such as already-exists and secret-exists remain policy decisions, and target-type, ownership, permission, alias, and identity failures remain safety-policy outcomes.

Compatibility

Consumers that caught path-alias around remove() will now receive not-found, not-empty, or not-removable. Consumers branching on FsSafeError.category will also observe not-found as operational package-wide. The changelog calls out both changes plainly and credits @Yigtwxx.

Downstream OpenClaw impact

Sequence the OpenClaw version bump with these changes:

• One test breaks: src/infra/fs-safe-remove.test.ts:66-74 asserts /ENOTEMPTY|EEXIST|EPERM/ against removePathWithinRoot; the surfaced code becomes not-empty.
• One workaround becomes dead: findPathAliasFilesystemCause() at src/infra/fs-safe-remove.ts:29-37 exists only to unwrap the errno from the former bogus path-alias.
• Three latent bugs are fixed: workspace-reconcile-derived-paths.ts:60-64, config/mutate.ts:542-546, and plugin-sdk/memory-host-core.ts:51 already check not-found/not-removable; those branches become reachable.

CI context

The intermittent red Windows legs on #84 are the sidecar-lock EPERM flake fixed separately by #92 (fix/sidecar-lock-native). This branch does not attempt to fix or absorb that unrelated change and remains based directly on the contributor's #84 history.

Verification

• origin/main regression replay: expected failures reproduced for policy vs operational and path-alias vs not-found
• focused tests: 2 files passed, 37 tests passed
• pnpm check: 59 files passed, 1 skipped; 650 tests passed, 25 skipped; package check passed
• pnpm test:security: 5 files passed; 64 tests passed
• git diff --check
• shared Codex autoreview against origin/main: clean, no accepted/actionable findings

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 5:26 PM ET / 21:26 UTC.

ClawSweeper review

What this changes

This PR makes Root.remove() report documented operational deletion failures (not-found, not-empty, and not-removable) while preserving fail-closed errors for ambiguous parent-directory guard failures.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

The target-deletion mapping appears correct and is narrowly covered; keep this PR open for a maintainer to explicitly accept the package-wide not-found category compatibility change before merge.

Priority: P2
Reviewed head: bfea4099287e9862a3c7524706de21c74d01c0f9
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, well-documented repair with meaningful regression coverage and real-run validation; the remaining merge gate is the intentional public category migration.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The PR body includes after-fix terminal validation from a real Windows setup, a current-main regression replay, focused test totals, full pnpm check, and pnpm test:security; the evidence directly covers the changed error outcomes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body includes after-fix terminal validation from a real Windows setup, a current-main regression replay, focused test totals, full pnpm check, and pnpm test:security; the evidence directly covers the changed error outcomes.
Evidence reviewed	6 items	Current-main defect: Current removePathInRoot() catches every fallback failure and passes it to normalizePinnedPathError(), which converts non-FsSafeError filesystem errors into path-alias; this conflicts with the documented remove codes. Published contract: The error reference already documents not-empty for removing non-empty directories, not-found for absent targets, and not-removable for other unlink/rmdir failures. Scoped repair preserves the guard boundary: The PR maps errno-shaped errors only around target lstat/rmdir/rm; the parent guard has a separate normalizer that leaves FsSafeError intact and sends ambiguous raw guard failures through the existing fail-closed path-alias path.
Findings	None	None.
Security	None	None.

How this fits together

Root is the package's capability-style filesystem boundary: callers submit an untrusted relative path, the package validates containment and directory identity, then performs the requested mutation. This PR changes the error contract emitted after guarded target deletion so applications can distinguish routine deletion outcomes from safety-policy failures.

flowchart LR
  Caller[Application caller] --> Root[Root capability]
  Root --> Resolve[Containment and path resolution]
  Resolve --> Guard[Parent directory identity guard]
  Guard --> Delete[Delete target path]
  Delete --> Normalize[Map deletion error]
  Normalize --> Result[Public FsSafeError code]

Loading

Decision needed

Question	Recommendation
Should the next release intentionally reclassify every exported not-found error from policy to operational, as this PR documents?	Accept the operational category: Merge the documented migration so routine absence is consistently operational across all APIs that emit not-found.

Why: The code-to-category mapping is exported and applies beyond Root.remove(), so source review can establish consistency but cannot choose the package's compatibility contract on behalf of release owners.

Before merge

• Resolve merge risk (P1) - Merging changes the exported FsSafeError.category for every existing not-found error from policy to operational; consumers that branch on category rather than the documented code need an intentional compatibility decision.
• Complete next step (P2) - No mechanical repair remains; a release owner must decide whether the exported package-wide category change is acceptable before merge.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch surface	9 files affected; 165 added, 11 removed	The implementation, exported error semantics, focused tests, documentation, and changelog move together for one public contract change.
Focused regression coverage	2 test files changed	Coverage exercises target errno mapping, guard-stage fail-closed handling, and real-disk missing/non-empty removal outcomes.

Root-cause cluster

Relationship: canonical
Canonical: #93
Summary: This PR contains the full corrective history from the original remove-error proposal and adds the explicit category-contract decision needed for the complete fix.

Members:

• superseded: fix(root): report documented remove() failure codes #84 - Its two corrective commits are ancestors of this branch; this PR carries them forward and adds category, documentation, changelog, and regression work.

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

Merge-risk options

Maintainer options:

1. Accept the documented category migration (recommended)
   Approve the package-wide not-found classification change and retain the Unreleased migration guidance for callers that branch on error category.
2. Narrow the change to deletion codes
   Keep the remove syscall mappings but preserve the existing category for pre-existing not-found errors if that compatibility contract is preferred.

Technical review

Best possible solution:

Merge the scoped deletion-error repair only after explicitly accepting not-found as an operational code package-wide, keeping the changelog migration note and guard-stage fail-closed behavior intact.

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

Yes—current main's catch path deterministically normalizes raw deletion failures to path-alias, while the existing public error reference specifies not-found, not-empty, and not-removable for those outcomes. The PR also supplies focused real-disk regression cases for missing targets and non-empty directories.

Is this the best way to solve the issue?

Yes for the remove behavior: the mapping is limited to target deletion syscalls, while parent-directory guard failures remain fail-closed. The remaining question is not technical correctness but whether the package-wide not-found category migration is the intended compatibility contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2477f5681f68.

Labels

Label changes:

• add P2: This is a bounded correction to a released public filesystem error contract with limited but real downstream behavior impact.
• add merge-risk: 🚨 compatibility: Existing consumers using path-alias around remove or branching on not-found category can observe changed results after upgrade.
• add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal validation from a real Windows setup, a current-main regression replay, focused test totals, full pnpm check, and pnpm test:security; the evidence directly covers the changed error outcomes.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal validation from a real Windows setup, a current-main regression replay, focused test totals, full pnpm check, and pnpm test:security; the evidence directly covers the changed error outcomes.

Label justifications:

• P2: This is a bounded correction to a released public filesystem error contract with limited but real downstream behavior impact.
• merge-risk: 🚨 compatibility: Existing consumers using path-alias around remove or branching on not-found category can observe changed results after upgrade.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal validation from a real Windows setup, a current-main regression replay, focused test totals, full pnpm check, and pnpm test:security; the evidence directly covers the changed error outcomes.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal validation from a real Windows setup, a current-main regression replay, focused test totals, full pnpm check, and pnpm test:security; the evidence directly covers the changed error outcomes.

Evidence

What I checked:

• Current-main defect: Current removePathInRoot() catches every fallback failure and passes it to normalizePinnedPathError(), which converts non-FsSafeError filesystem errors into path-alias; this conflicts with the documented remove codes. (src/root-impl.ts:989, 2477f5681f68)
• Published contract: The error reference already documents not-empty for removing non-empty directories, not-found for absent targets, and not-removable for other unlink/rmdir failures. (docs/errors.md:100, 2477f5681f68)
• Scoped repair preserves the guard boundary: The PR maps errno-shaped errors only around target lstat/rmdir/rm; the parent guard has a separate normalizer that leaves FsSafeError intact and sends ambiguous raw guard failures through the existing fail-closed path-alias path. (src/root-impl.ts:1329, bfea4099287e)
• Public category migration is explicit: The exported categorizer assigns the three routine removal codes to operational; the PR updates the error reference and Unreleased changelog to disclose that all existing not-found instances change category package-wide. (src/errors.ts:29, bfea4099287e)
• Current behavior provenance: Blame attributes the current remove wrapper and operational-code table to the v0.5.1 release commit, so this PR is correcting the present released implementation rather than reverting a later main-only change. (src/root-impl.ts:991, 16e1bd489ae8)
• Related-PR relationship: The original corrective branch is an ancestor of this PR; this continuation retains its guard-stage correction and adds the category decision, public documentation, changelog, and categorizer coverage. (bfea4099287e)

Likely related people:

• steipete: Authored the current v0.5.1 baseline containing the affected remove wrapper and the PR commit that makes the package-wide category decision. (role: current root/error-contract contributor; confidence: high; commits: 16e1bd489ae8, bfea4099287e; files: src/root-impl.ts, src/errors.ts, CHANGELOG.md)
• Yigtwxx: Authored the two retained commits that separate parent-guard failures from target-deletion errno mapping and add focused regression coverage. (role: original corrective implementation contributor; confidence: high; commits: 44cc2064931e, e96eb9c4b267; files: src/root-errors.ts, src/root-impl.ts, test/edge-coverage.test.ts)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.

[clawsweeper]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(root): report operational remove failure codes This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.