Skip to content

fix(plugins): clear hidden npm lock during peer repair#78265

Closed
Patrick-Erichsen wants to merge 2 commits into
fix-plugin-peer-root-repairfrom
pe/pr-78251-managed-root-prune
Closed

fix(plugins): clear hidden npm lock during peer repair#78265
Patrick-Erichsen wants to merge 2 commits into
fix-plugin-peer-root-repairfrom
pe/pr-78251-managed-root-prune

Conversation

@Patrick-Erichsen

@Patrick-Erichsen Patrick-Erichsen commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • repair stale managed-root openclaw peer state with npm-owned cleanup first: npm uninstall openclaw when the root manifest still lists it, otherwise npm prune for lock/disk residue
  • keep direct cleanup as a fallback/backstop, including npm's hidden node_modules/.package-lock.json
  • extend managed-root and npm-spec install tests to cover the cleanup path

Verification

  • pnpm test src/infra/npm-managed-root.test.ts src/plugins/install.npm-spec.test.ts
  • pnpm exec 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
  • Crabbox/Blacksmith pnpm test src/infra/npm-managed-root.test.ts src/plugins/install.npm-spec.test.ts (tbx_01kqxs1ahp3ggp95mwrqnh5eb0, exit 0)

Stacks on #78251.

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

clawsweeper Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
The PR updates the stacked managed npm root peer-repair helper to run npm uninstall/prune before direct cleanup, delete npm's hidden node_modules/.package-lock.json, pass timeout/logger through npm plugin install, and extend regression tests.

Reproducibility: Do we have a high-confidence way to reproduce the issue? No: source inspection and npm's hidden-lockfile contract make the stale-cache failure plausible, but there is no live current-branch managed npm install trace showing the hidden lock preserving stale peer state.

Real behavior proof
Needs real behavior proof before merge: The PR body provides tests, formatter output, and Testbox test execution only; it needs terminal/log/live output from a real managed npm plugin install or repair scenario before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Contributor/maintainer follow-up is needed for real behavior proof and the stacked base; there is no narrow ClawSweeper repair defect to queue.

Security
Cleared: The diff changes a managed npm repair path and tests without adding dependencies, workflows, lifecycle scripts, permissions, downloaded artifacts, or secret handling.

Review details

Best possible solution:

Land or fold this hidden-lock cleanup with the managed-root peer repair stack once #78251 is ready, with real managed npm install or repair proof attached.

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

Do we have a high-confidence way to reproduce the issue? No: source inspection and npm's hidden-lockfile contract make the stale-cache failure plausible, but there is no live current-branch managed npm install trace showing the hidden lock preserving stale peer state.

Is this the best way to solve the issue?

Is this the best way to solve the issue? Yes as a code direction: using npm cleanup first and deleting the hidden lockfile after direct node_modules cleanup is narrow and aligned with npm guidance, but merge should wait for the stacked base and real behavior proof.

What I checked:

Likely related people:

  • vincentkoc: Introduced the stacked peer-repair base in fix(plugins): repair stale managed openclaw peers #78251 and has several recent current-main commits on managed npm/plugin install repair behavior. (role: adjacent owner; confidence: high; commits: 97c68301d2fe, ba3c0fc78e83, d7dbf1150432; files: src/infra/npm-managed-root.ts, src/infra/npm-managed-root.test.ts, src/plugins/install.ts)
  • steipete: Recently refactored file-access helpers and maintained plugin install/docs/test surfaces touched by this PR. (role: recent maintainer; confidence: medium; commits: b85b1c68d175, 538605ff44d2, 41bbc4c048f2; files: src/infra/npm-managed-root.ts, src/plugins/install.ts, src/plugins/install.npm-spec.test.ts)
  • Lucenx9: Authored the current main managed npm root pinning helper history that this stacked repair extends. (role: introduced adjacent behavior; confidence: medium; commits: fb3ee066d8cb; files: src/infra/npm-managed-root.ts)

Remaining risk / open question:

  • The PR is stacked on open/draft fix(plugins): repair stale managed openclaw peers #78251, so final main-branch behavior depends on that base landing or this patch being rebased cleanly.
  • The PR body still lacks after-fix live output from a real managed npm plugin install or repair scenario.

Codex review notes: model gpt-5.5, reasoning high; reviewed against be1c99b76a59.

@steipete

steipete commented May 6, 2026

Copy link
Copy Markdown
Contributor

Superseded by #78291, which is now merged on main as 8e533490ab0a94fec8b3bdb3b184ebb5ce2edaba.

The merged canonical fix includes the hidden-lock/root-poison cleanup plus the broader managed npm peer repair and regression coverage, so this narrower PR is no longer needed. Closing to avoid duplicate review paths.

@steipete steipete closed this May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants