Skip to content

Repair active npm host after plugin installs#87573

Open
mbelinky wants to merge 2 commits into
mainfrom
mb/fix-managed-npm-root-host-prune
Open

Repair active npm host after plugin installs#87573
mbelinky wants to merge 2 commits into
mainfrom
mb/fix-managed-npm-root-host-prune

Conversation

@mbelinky

@mbelinky mbelinky commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Detect when a managed plugin npm install prunes dependencies from the active global-prefix OpenClaw package.
  • Repair the active OpenClaw package at the same installed version before reporting plugin install success, including required dependencies and optional runtime dependencies that were present before the install.
  • Roll back plugin installs if active-host repair fails, and keep npm lifecycle scripts disabled during repair.

Verification

  • node scripts/run-vitest.mjs src/plugins/install.npm-spec.test.ts src/infra/npm-managed-root.test.ts
  • ./node_modules/.bin/oxfmt --check --threads=1 src/infra/npm-managed-root.ts src/infra/npm-managed-root.test.ts src/plugins/install.ts src/plugins/install.npm-spec.test.ts
  • git diff --check
  • Testbox-through-Crabbox: tbx_01kspspe3ktdsxrpmg7b24aqtv

Real behavior proof
Behavior addressed: Installing an external plugin into the same managed npm prefix as a global OpenClaw package can leave the active OpenClaw package missing runtime dependencies.
Real environment tested: Blacksmith Testbox through Crabbox with a PR-built OpenClaw tarball installed into a throwaway global npm prefix, then the published Twilio WhatsApp plugin installed into that same managed prefix.
Exact steps or command run after this patch: Built and packed this branch, installed the tarball globally with npm --prefix, ran openclaw plugins install npm:@srinathh/[email protected] --dangerously-force-unsafe-install, verified active host dependencies, then ran openclaw doctor --fix.
Evidence after fix: The Testbox run printed json5=2.2.3, croner=10.0.1, listed twilio-whatsapp in plugins, and ended with PROOF_OK active host deps survived plugin install and doctor ran.
Observed result after fix: Testbox-through-Crabbox tbx_01kspspe3ktdsxrpmg7b24aqtv completed with exit=0. Focused local tests also passed: 2 test files, 58 tests.
What was not tested: The production gateway was not restarted or repointed to this PR. AWS Crabbox did not execute because AWS lease creation hit a security-group rule limit, so the remote proof used Blacksmith Testbox through Crabbox.

@openclaw-barnacle openclaw-barnacle Bot added size: M maintainer Maintainer-authored PR labels May 28, 2026
@clawsweeper

clawsweeper Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 4:15 PM ET / 20:15 UTC.

Summary
This PR adds active-host dependency snapshot/repair after managed npm plugin installs and changes unchanged npm plugin updates to run installer-backed maintenance before reporting unchanged.

PR surface: Source +308, Tests +467. Total +775 across 6 files.

Reproducibility: Partly. The active-host failure has PR-body Testbox live-output proof, and the PR-introduced unchanged-update behavior change is source-reproducible from the diff against current main; I did not run fresh commands in this read-only review.

Review metrics: 1 noteworthy metric.

  • Unchanged npm update fast path: 1 changed. The PR turns the current no-installer unchanged path into live installer-backed maintenance, which is compatibility-sensitive before merge.

Stored data model
Persistent data-model change detected: serialized state: src/infra/npm-managed-root.test.ts, serialized state: src/plugins/install.npm-spec.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Refresh or replace the branch against current main.
  • Remove the unchanged-update installer call or get explicit maintainer approval plus redacted live upgrade/failure-mode proof.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body gives useful Testbox live-output proof for active-host dependency survival, but not for the added unchanged-update installer behavior on a refreshed, non-conflicting head. 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.

Risk before merge

  • [P1] The PR is currently conflicting with main, so the exact merge result still needs a refreshed branch or replacement before normal review can clear it.
  • [P1] The unchanged npm update fast path would become live installer-backed maintenance, which can make already up-to-date plugin updates fail on installer, integrity, rollback, network, or config-reconciliation work that current users avoid.
  • [P1] The active-host repair may still be useful, but it must be reconciled with current per-plugin npm project roots rather than landing stale shared-root assumptions.

Maintainer options:

  1. Refresh Narrow Host Repair (recommended)
    Refresh or replace the branch so the active-host repair targets current main and drops the unchanged-update installer call.
  2. Approve Update Maintenance Explicitly
    If maintainers want unchanged npm updates to perform installer maintenance, require upgrade and failure-mode proof for that behavior before merge.
  3. Pause The Stale Branch
    If current per-plugin isolation already removes the original failure mode, pause or close this branch rather than landing broad update semantics.

Next step before merge

  • [P1] Maintainer handling is needed because the PR is protected, conflicting, and contains a compatibility decision rather than a narrow automation-safe repair.

Security
Cleared: No concrete security or supply-chain regression was found; the blocker is compatibility and availability around npm install/update behavior.

Review findings

  • [P1] Keep unchanged npm updates on the skip path — src/plugins/update.ts:1250-1267
Review details

Best possible solution:

Port only any still-needed active-host repair onto the current per-plugin npm project topology, and keep unchanged npm updates on the skip path unless maintainers explicitly approve and prove a separate maintenance mode.

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

Partly. The active-host failure has PR-body Testbox live-output proof, and the PR-introduced unchanged-update behavior change is source-reproducible from the diff against current main; I did not run fresh commands in this read-only review.

Is this the best way to solve the issue?

No. Active-host repair is a plausible mitigation, but this branch is not the best merge shape because it is conflicting and bundles an update-semantics compatibility change with the repair.

Full review comments:

  • [P1] Keep unchanged npm updates on the skip path — src/plugins/update.ts:1250-1267
    This turns the unchanged-artifact fast path into a live npm installer call. Current main skips reinstall when metadata says the artifact is unchanged; running installer/update work here can make already up-to-date plugins fail on network, integrity, rollback, or config reconciliation without a real update, so keep the skip path or split this into an explicitly approved compatibility change with upgrade proof.
    Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR targets a bounded managed npm plugin install reliability issue with compatibility concerns, not an emergency runtime outage.
  • merge-risk: 🚨 compatibility: The diff changes established unchanged npm update behavior and must be reconciled with current per-plugin install topology.
  • merge-risk: 🚨 availability: Failed host repair or added installer work can block plugin installs or updates and affect runtime dependency availability.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body gives useful Testbox live-output proof for active-host dependency survival, but not for the added unchanged-update installer behavior on a refreshed, non-conflicting head. 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.
Evidence reviewed

PR surface:

Source +308, Tests +467. Total +775 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 309 1 +308
Tests 3 476 9 +467
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 785 10 +775

What I checked:

  • Root policy applied: Root AGENTS.md was read fully; it requires deep PR review and treats plugin install/update compatibility changes as merge risk. (AGENTS.md:1, f0f5da0e3941)
  • Scoped plugin policy applied: The scoped plugin guide was read fully and confirms plugin install/update behavior is owned under the plugin control plane with compatibility-sensitive boundaries. (src/plugins/AGENTS.md:1, f0f5da0e3941)
  • Live PR state: Live GitHub metadata shows the PR is open, assigned to vincentkoc, labeled maintainer, and has mergeable=CONFLICTING / mergeStateStatus=DIRTY. (1e609cfb695c)
  • PR active-host repair path: The PR head adds repairManagedNpmRootActiveHostPackage, which repairs missing active host dependencies by running a global openclaw@<version> npm install under the managed prefix. (src/infra/npm-managed-root.ts:950, 1e609cfb695c)
  • PR unchanged-update change: The PR head calls installNpmSpecForUpdate inside the unchanged npm-artifact branch, so an already up-to-date npm plugin can now run installer/update work before returning unchanged. (src/plugins/update.ts:1250, 1e609cfb695c)
  • Current unchanged-update behavior: Current main keeps the unchanged path as a metadata/probe decision that pushes an unchanged outcome without running installNpmSpecForUpdate in that branch. (src/plugins/update.ts:1831, f0f5da0e3941)

Likely related people:

  • vincentkoc: They are the current assignee, authored the second PR commit that changes update semantics, and have recent current-main work in plugin update/install paths. (role: recent area contributor and assignee; confidence: high; commits: 1e609cfb695c, 767e8280ac7c, 97ce204d972c; files: src/plugins/update.ts, src/plugins/install.ts, src/infra/npm-managed-root.ts)
  • steipete: They authored and merged the current per-plugin npm install isolation PR that this stale branch must be reconciled with. (role: topology introducer and adjacent owner; confidence: high; commits: ea682182d05a, 5dc50b8a3f80, 463e9f270434; files: src/plugins/install.ts, src/plugins/install-paths.ts, src/plugins/npm-project-roots.ts)
  • zhuisDEV: They authored the merged official npm plugin install-record work adjacent to the update-record behavior this PR changes. (role: adjacent update contributor; confidence: medium; commits: 7b5f75eb98f4; files: src/plugins/update.ts, src/plugins/update.test.ts)
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.

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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 28, 2026
@clawsweeper

clawsweeper Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg: 🔥 warming; proof passed, review follow-up or readiness checks remain. Hatch with @clawsweeper hatch when eligible.

Rules and details

Hatchability:

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

About:

  • Eggs appear after real-behavior proof passes. They are collectible flavor only.
  • Review momentum changes the shell state: follow-up work warms it, re-review makes it wobble, and a clean final review lets it hatch.
  • 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.

@mbelinky
mbelinky marked this pull request as ready for review May 28, 2026 08:08
@BingqingLyu

This comment was marked as spam.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 28, 2026
@vincentkoc vincentkoc self-assigned this May 28, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels May 29, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 29, 2026
@clawsweeper clawsweeper Bot added 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: L 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.

4 participants