Skip to content

fix(openshell): limit container home path relocation to segments - #2211

Merged
freeqaz merged 1 commit into
openclaw:mainfrom
ericcaiwx-star:codex/batch-d-2-oct11
Oct 11, 2026
Merged

freeqaz merged 1 commit into
openclaw:mainfrom
ericcaiwx-star:codex/batch-d-2-oct11

Conversation

@ericcaiwx-star

Copy link
Copy Markdown
Contributor

Problem

Closes #2210

OpenShell rewrote every /home/node substring, corrupting values like /home/node_modules.

Change

See commit on ericcaiwx-star:codex/batch-d-2-oct11.

Verification

Recorded at base 3b36bff33006f4e4be262968b7435db57e0180e1 on USB worktree oce-batch-d-2. Focused node --test on the touched suite; exit 0.

Risks

None identified beyond the stricter validation or slug behavior described above.

Made with Cursor

Co-authored-by: Cursor <cursoragent@cursor.com>
@ericcaiwx-star
ericcaiwx-star marked this pull request as ready for review October 11, 2026 15:35
@clawsweeper

clawsweeper Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@ericcaiwx-star
ericcaiwx-star requested a review from a team as a code owner October 11, 2026 15:35
@ericcaiwx-star

Copy link
Copy Markdown
Contributor Author

Maintainer comment pass (author self-review):

  • One topic; owner-boundary fix only in the named module.
  • Regression test fails on pre-fix behavior at current main.
  • No config surface expansion, no secrets, no CHANGELOG edit.
  • Ready for ClawSweeper when CI attaches to the pushed head.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. 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. labels Oct 11, 2026
@clawsweeper

clawsweeper Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

What this changes

This PR preserves environment values such as /home/node_modules while continuing to relocate /home/node and its child paths.

Example: A Sandbox workload declares NODE_PATH=/home/node_modules.

  • Before: OpenShell receives NODE_PATH=/sandbox/.openclaw-runtime/home_modules.
  • After: OpenShell receives NODE_PATH=/home/node_modules.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) A useful, focused patch with meaningful regression coverage, but the required real-behavior proof is absent.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: An environment path containing a longer name such as /home/node_modules reaches the Harness with an incorrect location.
Reason: Preserving unrelated path names restores the existing relocation intent through a minimal owner-local change.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep this PR open: it makes a focused correction to a current-main OpenShell environment-mapping bug, with no introduced defect found.

Priority: P2
Reviewed head: 3647c5dca9e55a30529b206decf865b8c936107b

Before merge

  • Add real behavior proof - The changed OpenShell mapper is covered by a provisioning test with a mocked gateway client, and the contribution supplies only test claims; attach redacted terminal output or logs from the supported deployment workflow showing the preserved prefix path and relocated home path, excluding IPs, API keys, phone numbers, and non-public endpoints. 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.

Findings

None.

Tests

  • Missing end-to-end proof: Missing a supported OpenShell Agent deployment showing preserved /home/node_modules alongside relocated CODEX_HOME; the claimed base-fail/head-pass test comparison has no attached output.
Agent review details

How this fits together

Kubernetes Compute supplies Harness workload requirements to the OpenShell Sandbox Driver, which translates environment and mount paths into the outgoing CreateSandbox request.

flowchart LR
  A[Agent revision] --> B[Kubernetes Compute]
  B --> C[Harness requirements]
  C --> D[OpenShell path mapping]
  D --> E[CreateSandbox request]
  E --> F[Harness runtime]
Loading

Technical review

Best possible solution:

Retain this narrow correction in the existing OpenShell environment mapper.

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

The current-main mapper deterministically corrupts /home/node_modules; source and the regression assertion establish the failure path, but no reproduction was executed during this read-only review.

Is this the best way to solve the issue?

The patch corrects the owning Driver directly, preserves exact mount overrides and intended home relocation, and adds focused coverage without another mapping layer.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b7a4d3bfe0ff.

Provenance checked

  • OpenShell environment home relocation keeps the original intent (dd2b058: feat/openshell first agent codex #1400 introduced relocated writable runtime state for the first-Agent dedicated Codex workflow; its introducing diff explicitly added the unrestricted replacement.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • freeqaz: Raw commit 585083f adds apps/controller/src/drivers/sandbox/openshell.ts:285 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 585083f50100; files: apps/controller/src/drivers/sandbox/openshell.ts)

Root-cause cluster

Relationship: same_root_cause
Canonical: #2210
Summary: This PR repairs the exact environment-mapping failure described by the linked report.

Members:

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

Labels

Label changes:

  • add P2: Incorrect path rewriting can corrupt Harness environment values on the experimental OpenShell path, with limited scope.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

  • P2: Incorrect path rewriting can corrupt Harness environment values on the experimental OpenShell path, with limited scope.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach runtime evidence of the corrected environment through a supported OpenShell Agent deployment.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

Reviewed October 11, 2026, 11:40 AM ET / 15:40 UTC.

@freeqaz
freeqaz merged commit 03d494d into openclaw:main Oct 11, 2026
45 of 47 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: 🧂 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenShell corrupts environment values containing /home/node as a prefix

2 participants