Skip to content

fix(env): clone state containing Unix sockets - #109

Merged
hannesrudolph merged 1 commit into
openclaw:mainfrom
hannesrudolph:fix/clone-skip-unix-sockets-pr108
Aug 26, 2026
Merged

fix(env): clone state containing Unix sockets#109
hannesrudolph merged 1 commit into
openclaw:mainfrom
hannesrudolph:fix/clone-skip-unix-sockets-pr108

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Aug 25, 2026

Copy link
Copy Markdown
Member

Problem

ocm env clone could not clone a running environment when its state tree
contained a live Unix-domain socket.

The recursive state copier handled directories and symlinks specially, but
sent every other file type to fs::copy. A service-owned socket such as:

.openclaw/tailscale/tailscaled.sock

therefore caused the entire clone command to fail before the target environment
was registered. This blocked canary, recovery, and migration workflows for
otherwise healthy environments.

Why the socket should not be copied

A Unix socket is a live process endpoint, not durable environment data. Its
contents cannot be copied into another environment, and the owning service must
create a new socket when that environment starts.

The correct clone behavior is therefore:

  • copy regular files, directories, and symlinks with their existing semantics
  • omit Unix sockets
  • allow the cloned environment's services to recreate their own sockets

Fix

copy_path now detects Unix sockets with FileTypeExt::is_socket() and skips
them. A Unix-only regression test binds a real UnixListener, clones the
source tree, and verifies that:

  • an ordinary file is copied
  • the socket path is omitted
  • the recursive clone completes successfully

Closes #108

Testing

  • cargo test copy_dir_recursive_skips_unix_sockets -- --nocapture
  • cargo test --test env_clone_tests -- --nocapture
  • cargo test --test daemon_runtime_tests -- --test-threads=1 --nocapture
  • cargo test --workspace -- --test-threads=1 --skip bin_wrapper_runs_with_an_overridden_home
  • cargo build --release
  • cargo fmt --check
  • git diff --check

The local host lacks rustup, so its unrelated wrapper fixture was excluded
from the broad local run. GitHub CI ran the complete suite, including that
fixture, successfully on macOS and Ubuntu.

Live proof

  • before the fix, ocm env clone main main-ecf-canary --json failed on
    Main's live .openclaw/tailscale/tailscaled.sock
  • with the fix installed, the same clone completed without manually stopping
    Tailscale or deleting the socket
  • ordinary state was copied and the socket was omitted
  • all five production gateway PID, restart-count, and binding identities were
    unchanged immediately after clone and after supervisor convergence

Worked on by

  • Hannes Rudolph

@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@hannesrudolph hannesrudolph changed the title fix(env): skip Unix sockets when cloning state fix(env): clone state containing Unix sockets Aug 25, 2026
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 26, 2026
@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 7:36 PM ET / 23:36 UTC.

ClawSweeper review

What this changes

The PR skips Unix-domain sockets during recursive environment-state copies and adds a Unix-only test proving normal files still copy.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep this active, member-authored PR open for normal maintainer approval. Current main still sends Unix sockets to fs::copy; the patch narrowly corrects that path with realistic regression coverage.

Priority: P2
Reviewed head: cf062f9345c7d7df3ed81ca8a69d0cc03c9c2992

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, source-confirmed repair with targeted regression coverage and no correctness or security finding.
Proof confidence 🌊 off-meta tidepool Not applicable: Not applicable: this repository-member PR is exempt from the external contributor proof gate; its body nevertheless describes an after-fix live clone and the patch includes a real Unix-socket regression test.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Not applicable: this repository-member PR is exempt from the external contributor proof gate; its body nevertheless describes an after-fix live clone and the patch includes a real Unix-socket regression test.
Evidence reviewed 6 items Current-main failure path: Current main classifies symlinks and directories, then falls through to fs::copy for every other entry type; a Unix socket therefore reaches the unsupported regular-file copy path.
Clone integration: The standard environment clone copies the complete source root through the shared recursive helper before registering the target and clears runtime residue afterward.
Focused repair and regression coverage: The branch checks Unix socket file types before fs::copy and creates a real UnixListener to verify a copied regular file, omitted socket path, and successful recursion.
Findings None None.
Security None None.

Live Verification

Command: cargo test copy_dir_recursive_skips_unix_sockets -- --nocapture

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.24.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.24.0.tgz

Assertions:

  • FAIL expect_output: test store::common::tests::copy_dir_recursive_skips_unix_sockets ... ok

How this fits together

OCM environment lifecycle commands recursively copy durable state into a new environment before its services start. The shared copy helper feeds clone, import, migration, and simulation flows, which then create fresh runtime endpoints in the destination.

flowchart LR
A[Environment lifecycle command] --> B[Recursive state copy]
B --> C{Filesystem entry type}
C -->|Directory or link| D[Copy durable state]
C -->|Unix socket| E[Omit live endpoint]
D --> F[Destination environment]
E --> F
F --> G[Services create runtime endpoints]
Loading

Before merge

  • Complete next step (P2) - The PR is active and member-authored, so it requires ordinary maintainer merge judgment rather than automated cleanup or repair.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +6, tests +26 The 32-line single-file patch adds one platform-gated copy rule and a direct regression test without dependency or workflow changes.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #108
Summary: This PR is the direct candidate fix for the linked Unix-socket clone failure.

Members:

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

Technical review

Best possible solution:

Merge the focused socket-omission behavior after ordinary maintainer review, retaining the UnixListener regression test as the contract for live runtime endpoints.

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

Yes: on Unix, create a UnixListener inside a source tree and call the current recursive copier; current main's file-type fallthrough reaches fs::copy, while the branch test exercises the repaired path.

Is this the best way to solve the issue?

Yes: checking FileTypeExt::is_socket after existing symlink and directory handling preserves durable copy semantics while excluding a non-copyable live endpoint before fs::copy.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 035686953c68.

Labels

Label changes:

  • add P2: The PR repairs a bounded environment-clone failure that can interrupt canary, recovery, and migration workflows.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Not applicable: this repository-member PR is exempt from the external contributor proof gate; its body nevertheless describes an after-fix live clone and the patch includes a real Unix-socket regression test.

Label justifications:

  • P2: The PR repairs a bounded environment-clone failure that can interrupt canary, recovery, and migration workflows.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Not applicable: this repository-member PR is exempt from the external contributor proof gate; its body nevertheless describes an after-fix live clone and the patch includes a real Unix-socket regression test.

Evidence

What I checked:

  • Current-main failure path: Current main classifies symlinks and directories, then falls through to fs::copy for every other entry type; a Unix socket therefore reaches the unsupported regular-file copy path. (src/store/common.rs:90, 035686953c68)
  • Clone integration: The standard environment clone copies the complete source root through the shared recursive helper before registering the target and clears runtime residue afterward. (src/store/envs.rs:556, cf062f9345c7)
  • Focused repair and regression coverage: The branch checks Unix socket file types before fs::copy and creates a real UnixListener to verify a copied regular file, omitted socket path, and successful recursion. (src/store/common.rs:90, cf062f9345c7)
  • Documented lifecycle boundary: The usage guide states that clone produces a separate environment and clears copied runtime residue, consistent with omitting a live service endpoint. (docs/USAGE.md:434, cf062f9345c7)
  • Feature provenance: History identifies the recursive store-copy helper as originating in the March refactor; later history also includes focused symlink hardening in the same file. (src/store/common.rs:57, 8185e7b12390)
  • Release status: No tag contains the proposed commit, so the fix is not in the stated latest release v0.2.33. (cf062f9345c7)

Likely related people:

  • shakkerdroid: The recursive store-copy helper appears to date to the original refactor and subsequent maintenance in this file is predominantly by Shakker. (role: introduced shared copy helper; confidence: high; commits: 8185e7b12390, 147f9853e81b, 4f1e19b4a6b7; files: src/store/common.rs)
  • Vincent Koc: Vincent Koc authored prior symlink-safety hardening in the same shared helper. (role: adjacent copy-path contributor; confidence: medium; commits: fce19122169a; files: src/store/common.rs)

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.

@hannesrudolph
hannesrudolph merged commit cd8deca into openclaw:main Aug 26, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

env clone fails when source state contains a Unix socket

1 participant