Repair active npm host after plugin installs#87573
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 4:15 PM ET / 20:15 UTC. Summary 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.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest 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:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f0f5da0e3941. Label changesLabel justifications:
Evidence reviewedPR surface: Source +308, Tests +467. Total +775 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 PR egg: 🔥 warming; proof passed, review follow-up or readiness checks remain. Hatch with Rules and detailsHatchability:
About:
|
Summary
Verification
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.