Skip to content

fix: isolate runtime install lifecycle state - #131

Merged
shakkernerd merged 11 commits into
openclaw:mainfrom
MertBasar0:codex/ocm-98-runtime-install-repro
Sep 14, 2026
Merged

shakkernerd merged 11 commits into
openclaw:mainfrom
MertBasar0:codex/ocm-98-runtime-install-repro

Conversation

@MertBasar0

@MertBasar0 MertBasar0 commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Related: #98

What Problem This Solves

Resolves a problem where preparing an OpenClaw runtime lets npm installation scripts discover and modify the caller's existing OpenClaw state before the new runtime is recorded.

Why This Change Was Made

Package and companion installation now direct OpenClaw's home, state, and config selectors into disposable installation state and clear inherited service-state and active-environment selectors. The npm process retains the caller's home, configuration, and script policy. Installation state is removed when npm returns, including a normal failure.

User Impact

Preparing a runtime preserves caller OpenClaw state through the supported lifecycle path selectors. User npm settings, including disabled scripts, explicit configuration files, and paths expanded from HOME, continue to apply.

Evidence

Validation on Crabbox with Rust 1.88.0, Node.js 24.18.1, and npm 11.16.0:

  • Formatting, locked all-target compilation, JavaScript/shell syntax, and complete Rust test coverage passed, including 56 runtime tests and 40 service tests.
  • Real npm honored enabled/disabled scripts with both default and explicit configuration files, including an explicit file outside HOME. Enabled hooks retained HOME, USERPROFILE, effective userconfig, and HOME-expanded cache settings.
  • An intentionally failing hook produced no published runtime or installation residue; caller database, config, and npmrc bytes stayed unchanged.
  • The unchanged beta postinstall cleanup code executed with and without an inherited service-state selector. Both caller dependency sentinels remained intact.

The reproduction uses the unmodified schema-6 database created by published OpenClaw 2026.8.1-beta.1 and installs 2026.8.1-beta.2. Database inspection is read-only, and the explicit-path control uses the legacy installer.

Installer Database schema Database bytes Service-state sentinel
OCM v0.2.32 6 → 8 Changed Removed
Fixed OCM 6 → 6 Preserved Preserved
OCM v0.2.32 with explicit disposable paths 6 → 6 Preserved Preserved

Config bytes remained unchanged in all three cases. The hosted reproduction also passed.

All required CI jobs passed. The two optional macOS npm jobs reproduce the unchanged-main empty team-ID failure tracked by #216.

@clawsweeper

clawsweeper Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

🦞👀
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.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 31, 2026
@clawsweeper

clawsweeper Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 14, 2026, 6:10 AM ET / 10:10 UTC (Revision 10).

ClawSweeper review

What this changes

Runtime and companion installation redirect OpenClaw lifecycle state into disposable directories while preserving npm settings, with regression tests and a published-package reproduction workflow.

Merge readiness

✅ Ready for maintainer review

This PR remains necessary: current main and v0.2.46 retain the installation-state leak. The updated patch resolves the earlier npm-configuration and reproduction-lane findings, with convincing published-package proof and no remaining actionable defect found.

Priority: P1
Reviewed head: 7f1785b4cd17c810e86409a48bddaab824804bdb

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with strong real-package proof, resolved prior findings, and no remaining correctness blocker.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (linked_artifact): The supplied published-package comparison exercises OCM's real runtime installer on Ubuntu and shows preserved caller database, config, and service state after the fix; GitHub confirms the reproduction passed on the reviewed head. Real-npm compatibility and failure-path observations supplement that result. Artifact retrieval was blocked for this reviewer, without invalidating the supplied evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (linked_artifact): The supplied published-package comparison exercises OCM's real runtime installer on Ubuntu and shows preserved caller database, config, and service state after the fix; GitHub confirms the reproduction passed on the reviewed head. Real-npm compatibility and failure-path observations supplement that result. Artifact retrieval was blocked for this reviewer, without invalidating the supplied evidence.
Evidence reviewed 9 items Repository policy and review scope: Read the complete root AGENTS.md and contribution guidance. No nested AGENTS.md or maintainer-notes directory was found. Applied fixture isolation, credential protection, and release-safeguard guidance; no builds or tests were executed during this read-only review.
Current main and latest release still need the fix: Inspected the installer on fetched main and v0.2.46. Both execute npm without redirecting OpenClaw lifecycle state; the shared command environment helper only adjusts PATH. Neither contains this PR's isolation helper.
Release comparison: The same unisolated npm invocation is present in the supplied latest release, v0.2.46.
Findings None None.
Security None None.

How this fits together

OCM prepares OpenClaw packages before publishing runtimes for managed environments. Its npm installation boundary controls which state package lifecycle scripts discover and therefore whether preparation can affect an existing gateway.

flowchart TD
  A[Runtime installation request] --> B[Package and companion installer]
  C[Caller npm settings] --> B
  B --> D[Redirect OpenClaw state paths]
  D --> E[Run npm lifecycle scripts]
  E --> F[Remove disposable state]
  F --> G[Publish successful runtime]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth Production +40/-2; Rust tests +297/-0; reproduction scripts +344/-0 The small production boundary change is supported by compatibility regressions and a published-package comparison.

Technical review

Best possible solution:

Keep npm's existing configuration contract while confining supported OpenClaw lifecycle state discovery to disposable installation paths.

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

Yes: current-main source exposes caller state to npm lifecycle scripts, and the published-package comparison demonstrates the resulting mutation with the legacy installer. This review did not execute a current-main reproduction.

Is this the best way to solve the issue?

Yes: redirecting OpenClaw's supported state selectors at both npm installation boundaries addresses the first write while preserving existing npm policy and configuration.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P1: Runtime preparation can mutate an existing gateway's database before upgrade checkpoint protection applies.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The supplied published-package comparison exercises OCM's real runtime installer on Ubuntu and shows preserved caller database, config, and service state after the fix; GitHub confirms the reproduction passed on the reviewed head. Real-npm compatibility and failure-path observations supplement that result. Artifact retrieval was blocked for this reviewer, without invalidating the supplied evidence.
  • proof: sufficient: Contributor real behavior proof is sufficient. The supplied published-package comparison exercises OCM's real runtime installer on Ubuntu and shows preserved caller database, config, and service state after the fix; GitHub confirms the reproduction passed on the reviewed head. Real-npm compatibility and failure-path observations supplement that result. Artifact retrieval was blocked for this reviewer, without invalidating the supplied evidence.

Evidence

What I checked:

  • Repository policy and review scope: Read the complete root AGENTS.md and contribution guidance. No nested AGENTS.md or maintainer-notes directory was found. Applied fixture isolation, credential protection, and release-safeguard guidance; no builds or tests were executed during this read-only review. (AGENTS.md:1, 7f1785b4cd17)
  • Current main and latest release still need the fix: Inspected the installer on fetched main and v0.2.46. Both execute npm without redirecting OpenClaw lifecycle state; the shared command environment helper only adjusts PATH. Neither contains this PR's isolation helper. (src/store/runtimes.rs:922, 3f50e7c4a9a7)
  • Release comparison: The same unisolated npm invocation is present in the supplied latest release, v0.2.46. (src/store/runtimes.rs:922, fc9f330476d3)
  • Installation boundary and cleanup: The helper redirects three OpenClaw path selectors and removes inherited service, profile, and active-environment selectors without changing HOME or npm configuration. Both package and companion installers invoke it before npm and remove disposable state afterward; existing staging cleanup handles failed installations. (src/store/runtimes.rs:922, 7f1785b4cd17)
  • Earlier compatibility finding resolved: The current diff preserves HOME and USERPROFILE. Added real-npm coverage exercises scripts enabled and disabled with default and explicit userconfig files, including HOME-expanded cache paths; host and managed-node fixtures check caller state preservation. The earlier reviewed SHA was unavailable locally, so no historical unchanged-code attribution was inferred. (tests/runtime_command_tests.rs:2259, 7f1785b4cd17)
  • Real-package proof and dependency boundary: The complete supplied body at sourceRevision 7f071b59eb15558d61748860b20e8033a527dc6cf3b6a47e8c02d008c864e7d2 reports legacy schema 6→8 mutation versus fixed schema 6→6 with database, config, and service-state preservation. The inspected script installs published OpenClaw beta packages, establishing the relevant OpenClaw lifecycle dependency; its fixed lane supplies no OPENCLAW_* overrides and its explicit-path control uses the legacy binary. This proves the installation boundary, not every adoption safeguard requested in Adopted environment upgrade mutated the source gateway state database #98. (scripts/reproduce-ocm-98.sh:230, 7f1785b4cd17)

Likely related people:

  • shakkernerd: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • hannesrudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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.

History

Review history (9 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-31T22:02:40.613Z sha 402add4 :: needs changes before merge. :: [P1] Preserve npm configuration needed for dependency resolution
  • reviewed 2026-08-31T23:21:01.059Z sha 402add4 :: found issues before merge. :: [P1] Preserve npm configuration needed for dependency resolution
  • reviewed 2026-09-02T14:47:34.166Z sha 402add4 :: found issues before merge. :: [P1] Preserve npm configuration needed for dependency resolution
  • reviewed 2026-09-08T23:34:11.603Z sha 402add4 :: needs real behavior proof before merge. :: [P1] Preserve npm configuration needed for dependency resolution | [P2] Separate the fixed lane from the explicit-path workaround
  • reviewed 2026-09-11T00:00:45.189Z sha 8848cd1 :: needs real behavior proof before merge. :: [P1] Preserve npm configuration needed for dependency resolution
  • reviewed 2026-09-11T00:13:17.881Z sha 8848cd1 :: blocked before merge. :: [P1] Preserve npm configuration needed for dependency resolution
  • reviewed 2026-09-11T00:17:10.636Z sha 8848cd1 :: blocked before merge. :: [P1] Preserve npm configuration needed for dependency resolution
  • reviewed 2026-09-14T09:49:09.724Z sha 7f1785b :: needs maintainer review before merge. :: none

@MertBasar0

Copy link
Copy Markdown
Contributor Author

@steipete ClawSweeper accepted the supplied behavior proof but raised a P1 compatibility decision: the current isolation boundary changes npm's HOME, so npm no longer discovers an operator's home-level .npmrc. That can affect custom registries, proxies, certificates, or authenticated package sources.

Could you confirm the intended contract for this PR?

  1. Preserve supported npm transport configuration through a reviewed boundary while keeping caller-owned OpenClaw paths unavailable to lifecycle scripts; or
  2. Keep strict empty-home isolation and explicitly require/document another configuration path.

I lean toward option 1, but forwarding the original user-config path wholesale could also expose authentication material to package lifecycle scripts, so I do not want to assume that security/compatibility tradeoff without maintainer direction. Once confirmed, I can align the implementation and add focused regression coverage.

ClawSweeper finding: #131 (comment)

@MertBasar0

Copy link
Copy Markdown
Contributor Author

@steipete A quick follow-up on the npm configuration question above: could you confirm the preferred approach for preserving supported npm transport settings while keeping caller-owned OpenClaw state isolated?

I'm ready to update the implementation and add focused compatibility coverage once the direction is clear. If another maintainer owns this decision, happy to follow up with them. Thanks!

@MertBasar0

Copy link
Copy Markdown
Contributor Author

@fuller-stack-dev Could you take a look at the maintainer decision requested in the ClawSweeper review? The isolation fix and regression proof are complete; the remaining question is how OCM should preserve supported npm transport configuration without exposing caller-owned OpenClaw state to lifecycle scripts. Your guidance on the intended contract would unblock the final implementation update.

@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Corrected caller-mode real-package proof is complete on exact head 8848cd1a016ccc25e7977d16aafb0579e2dd851d:

  • OCM v0.2.32, caller environment with no explicit OPENCLAW_* overrides: schema 7 → 8; source DB changed.
  • Fixed current OCM, the same caller environment with no explicit OPENCLAW_* overrides: schema 7 → 7; source DB and config hashes unchanged.
  • Explicit-path control: schema 7 → 7; source DB and config hashes unchanged.

Formatting, all-target checks, the focused host/managed-npm regression, the full test suite, both release builds, the corrected published-package reproduction, and evidence upload all passed in https://github.com/MertBasar0/ocm/actions/runs/34544350861. The PR body now records the corrected command boundary and redacted result matrix.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 11, 2026
@shakkernerd shakkernerd self-assigned this Sep 14, 2026
@shakkernerd
shakkernerd force-pushed the codex/ocm-98-runtime-install-repro branch from 8848cd1 to 7f1785b Compare September 14, 2026 09:44
@clawsweeper clawsweeper Bot added 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. and removed merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 14, 2026
@shakkernerd
shakkernerd merged commit 3cb61a7 into openclaw:main Sep 14, 2026
8 of 10 checks passed
@shakkernerd

Copy link
Copy Markdown
Member

Fixed: runtime installation isolates OpenClaw lifecycle state while preserving the caller's npm configuration and script policy.

With thanks to @MertBasar0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. 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.

2 participants