Skip to content

fix(plugins): avoid spurious npm spec error after metadata failure#96143

Merged
RomneyDa merged 2 commits into
openclaw:mainfrom
brokemac79:codex/recreate-95581-plugin-update-metadata-failure
Jun 26, 2026
Merged

fix(plugins): avoid spurious npm spec error after metadata failure#96143
RomneyDa merged 2 commits into
openclaw:mainfrom
brokemac79:codex/recreate-95581-plugin-update-metadata-failure

Conversation

@brokemac79

@brokemac79 brokemac79 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Closes #77616

Recreates and repairs #95581.

What Problem This Solves

Fixes an issue where a transient npm metadata lookup failure for a range-style installed plugin spec could produce a confusing second error about an unsupported npm spec.

Why This Change Was Made

When metadata probing fails for a registry spec that the installer cannot validate, the update path now records the metadata check failure through the existing failure handler and stops before invoking the installer. That preserves disableOnFailure cleanup for post-core updates. Valid exact, tag, and bare registry specs keep the existing installer fallback behavior. The same change also normalizes managed extensions-root path comparison so updater tests pass consistently on Windows and POSIX paths.

User Impact

Users see the real metadata/registry failure instead of a misleading follow-on npm spec error, and failed post-core plugin updates still disable and clean stale plugin policy/slot references when configured to do so.

Evidence

  • node scripts/run-vitest.mjs src/plugins/update.test.ts passed locally: 99 tests.
  • git diff --check passed locally.
  • Testbox changed-scope proof passed on Blacksmith Testbox through direct SSH:
env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 corepack pnpm check:changed
  • result summary:
TESTBOX_HEAD=3f5af2643cae4622f980786426a05893e06badb0
[check:changed] lanes=core, coreTests
[check:changed] src/plugins/update.test.ts: core test
[check:changed] src/plugins/update.ts: core production
No guarded extension wildcard re-exports found.
No plugin-sdk wildcard re-exports found in extension API barrels.
PASS direct dependency pin guard: checked 470 directly declared dependency specs across 158 tracked package manifests; 0 violations.
PASS package patch guard: no new pnpm patches; 4 legacy patches allowlisted.
Database-first legacy-store guard passed.
runtime-sidecar-loaders: local runtime sidecar loaders look OK.
Import cycle check: 0 runtime value cycle(s).
TESTBOX_EXIT=0
  • Testbox note: direct SSH was used because the Windows Crabbox/Blacksmith wrapper could not resolve the linked worktree (could not resolve repo: could not get git remote URL: exit status 128). The Testbox proof avoids local sync by checking out the PR branch directly on the Testbox.
  • Azure Crabbox changed gate passed on the pushed head d6c9fbadacafbdfa50ee01b825413e212f8bc20f:
Provider: azure
Lease: cbx_dc2f963e94fd / slug crimson-krill
Run: run_71edbffb2fbe
Machine: Standard_D32ads_v6
Command: env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 corepack pnpm check:changed
Result: exitCode=0
Timing: sync 5m6.672s, command 5m30.692s, total 12m53.458s
Cleanup: one-shot lease stopped by Crabbox (leaseStopped=true)

The Azure changed gate included guarded extension wildcard re-export checks, plugin-sdk wildcard re-export checks, dependency pin guard, npm shrinkwrap guard, runtime sidecar baseline/owner test, package patch guard, database-first legacy-store guard, media download helper guard, runtime sidecar loader guard, all changed-scope typecheck, lint shards, and runtime import cycle check.

  • Autoreview: python .agents/skills/autoreview/scripts/autoreview --mode commit --commit HEAD --codex-bin %TEMP%/codex-fast-autoreview.cmd reported clean, no accepted/actionable findings.

Real behavior proof: plugin update metadata failure

Behavior or issue addressed: a range-style installed npm plugin spec that hits a transient metadata lookup failure should report the registry failure and stop, instead of falling through to the installer and adding a misleading unsupported npm spec error.

Real environment tested: Docker-backed Crabbox provider=local-container, image mcr.microsoft.com/playwright:v1.60.0-noble, lease cbx_32e734196d8b, slug quick-krill, PR head d6c9fbadacafbdfa50ee01b825413e212f8bc20f.

Exact steps or command run after this patch:

C:\Users\marti\.local\bin\crabbox.exe run --provider local-container --local-container-image mcr.microsoft.com/playwright:v1.60.0-noble --no-hydrate --timing-json --script C:\oc-work\lsr911-recreate\95581\.tmp\proof-96143-local-container.sh

The proof script installed dependencies, created an installed plugin record for @martian-engineering/lossless-claw@^0.9.0, routed production npm view subprocess calls to a local fake npm executable that returned registry timeout, then invoked the production post-core updater function updateNpmInstalledPlugins with disableOnFailure: true. The fake npm executable exits with a sentinel error for any install/add command, so the proof fails if the updater falls through to the installer path.

Evidence after fix:

PROOF_96143_RESULT={
  "scenario": "post-core plugin update after npm metadata lookup failure",
  "inputSpec": "@martian-engineering/lossless-claw@^0.9.0",
  "installedVersion": "0.9.0",
  "npmViewFailure": "registry timeout",
  "fakeNpmLog": [
    "FAKE_NPM argv=view @martian-engineering/lossless-claw@^0.9.0 name version dist.integrity dist.shasum openclaw --json"
  ],
  "warnings": [
    "Disabled \"lossless-claw\" after plugin update failure; OpenClaw will continue without it. Failed to check lossless-claw: npm view failed: registry timeout"
  ],
  "outcomes": [
    {
      "pluginId": "lossless-claw",
      "status": "skipped",
      "message": "Disabled \"lossless-claw\" after plugin update failure; OpenClaw will continue without it. Failed to check lossless-claw: npm view failed: registry timeout"
    }
  ],
  "configAfterFailure": {
    "allow": ["keep"],
    "deny": ["blocked"],
    "slots": {
      "memory": "memory-core",
      "contextEngine": "legacy"
    },
    "entry": {
      "enabled": false,
      "config": { "preserved": true }
    }
  },
  "installFallbackAttempted": false,
  "misleadingUnsupportedNpmSpecMessagePresent": false
}

Observed result after fix: the updater called only npm view, reported Failed to check lossless-claw: npm view failed: registry timeout, applied the disableOnFailure cleanup, did not attempt npm install/npm add, and did not emit any unsupported npm spec follow-on message.

Supplemental validation from the same local-container run:

Provider: local-container
Lease: cbx_32e734196d8b / slug quick-krill
Image: mcr.microsoft.com/playwright:v1.60.0-noble
Command phases: install 1m2.786s, behavior-proof 2.475s, changed-gate 13m40.814s
Changed gate command: env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 corepack pnpm check:changed
Changed gate result: exitCode=0
Timing: sync 3m22.014s, command 14m46.599s, total 18m8.710s
Cleanup: one-shot lease stopped by Crabbox (leaseStopped=true)

What was not tested: a live npm registry outage was not induced against npmjs.org; the proof uses a controlled failing npm view subprocess to exercise the real OpenClaw post-core plugin updater branch deterministically.

@brokemac79
brokemac79 marked this pull request as ready for review June 23, 2026 16:08
@brokemac79
brokemac79 marked this pull request as draft June 23, 2026 16:09
@brokemac79
brokemac79 marked this pull request as ready for review June 23, 2026 16:11
@brokemac79
brokemac79 force-pushed the codex/recreate-95581-plugin-update-metadata-failure branch from 3f5af26 to d6c9fba Compare June 23, 2026 19:31
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 7:41 PM ET / 23:41 UTC.

Summary
The PR changes npm plugin update handling so metadata failures for installer-rejected registry specs record the metadata error through failure cleanup instead of falling through to npm reinstall, with focused regression tests and path normalization for managed plugin roots.

PR surface: Source +4, Tests +101. Total +105 across 2 files.

Reproducibility: yes. at source level: current main logs metadata failure and falls through to installer validation, while installer validation rejects range-style registry specs before another metadata lookup. I did not induce a live npmjs.org outage during this read-only review.

Review metrics: 1 noteworthy metric.

  • Fallback behavior changed: 1 metadata-failure branch redirected. Plugin update fallback behavior runs during user-triggered and post-core update flows, so the changed branch needs maintainer-visible compatibility review.

Stored data model
Persistent data-model change detected: vector/embedding metadata: src/plugins/update.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #77616
Summary: This PR is the current candidate fix for the open plugin update metadata-failure diagnostic issue and supersedes the earlier closed unmerged attempt.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P2] The PR intentionally changes npm plugin update fallback behavior for existing installer-rejected specs after metadata lookup failure; maintainers should accept that compatibility tradeoff before merge.
  • [P2] The linked issue also discusses broader npm metadata retry/no-output-timeout handling, which this focused diagnostic and cleanup PR does not implement.

Maintainer options:

  1. Accept the focused fallback change (recommended)
    Maintainers can land this as a narrow diagnostic fix because installer-rejected specs would fail the fallback path anyway and the PR preserves disableOnFailure cleanup.
  2. Require metadata retry first
    If maintainers want the linked issue solved only by retry or no-output-timeout policy, ask for resolver-level work before merging this branch.
  3. Pause if fallback policy is unsettled
    If the fallback behavior is still an open product decision, keep the PR unmerged until the update policy is confirmed.

Next step before merge

  • No contributor-facing code defect remains; the next action is maintainer review of the compatibility-sensitive fallback tradeoff and normal exact-head merge validation.

Security
Cleared: The diff changes plugin updater control flow and tests only; it adds no dependency, workflow, package source, permission, generated artifact, or secret-handling surface.

Review details

Best possible solution:

Land the focused guard once maintainers accept the fallback tradeoff, and keep broader npm metadata retry/no-output handling as separate follow-up work if still desired.

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

Yes, at source level: current main logs metadata failure and falls through to installer validation, while installer validation rejects range-style registry specs before another metadata lookup. I did not induce a live npmjs.org outage during this read-only review.

Is this the best way to solve the issue?

Yes for the targeted symptom: the PR routes only installer-rejected specs through centralized failure handling and keeps valid exact, tag, and bare npm specs on the existing installer fallback. Broader retry/no-output behavior is a separate policy choice.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority plugin update diagnostic and cleanup fix with intermittent, limited-blast-radius user impact.
  • merge-risk: 🚨 compatibility: The diff changes npm plugin update fallback behavior for existing installer-rejected specs when registry metadata probing fails.
  • 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 (terminal): The PR body includes terminal output from a local-container Crabbox run exercising the production post-core updater branch and showing no installer fallback or unsupported-spec follow-on message.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a local-container Crabbox run exercising the production post-core updater branch and showing no installer fallback or unsupported-spec follow-on message.
Evidence reviewed

PR surface:

Source +4, Tests +101. Total +105 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 1 +4
Tests 1 103 2 +101
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 108 3 +105

What I checked:

  • Repository policy applied: Root policy treats plugin fallback behavior as compatibility-sensitive, and scoped plugin guidance owns plugin discovery, install/update, registry, and contract enforcement. (AGENTS.md:31, 0da26499dac2)
  • Current main still falls through after metadata failure: Current main calls resolveNpmSpecMetadata and logs that it is falling back to the installer path when metadata probing fails. (src/plugins/update.ts:1503, 0da26499dac2)
  • Failure cleanup path is centralized: recordFailure applies disableOnFailure cleanup, emits the disabled warning, mutates plugin config, and records skipped/error outcomes. (src/plugins/update.ts:1264, 0da26499dac2)
  • Post-core caller opts into cleanup: The post-core update path calls updateNpmInstalledPlugins with disableOnFailure: true, so metadata-failure behavior affects upgrade recovery. (src/cli/update-cli/update-command.ts:1880, 0da26499dac2)
  • Installer validation explains the misleading follow-on: installPluginFromNpmSpec validates the registry spec before its own metadata lookup, and the parser rejects ranges with the unsupported-spec message this PR avoids surfacing as a second error. (src/plugins/install.ts:3003, 0da26499dac2)
  • Proposed guard preserves cleanup for rejected specs: The PR head checks parseRegistryNpmSpec before installer fallback, records the metadata failure through recordFailure for rejected specs, and continues. (src/plugins/update.ts:1587, 440354ba3dd9)

Likely related people:

  • vincentkoc: Recent history ties this handle to official plugin recovery and npm registry spec hardening that the proposed guard relies on. (role: recent update and npm spec contract contributor; confidence: high; commits: 767e8280ac7c, f392b81e9502, f5a7f613ee4d; files: src/plugins/update.ts, src/plugins/update.test.ts, src/infra/npm-registry-spec.ts)
  • steipete: Recent history shows repeated work on npm metadata utilities, install-source handling, and plugin install/update behavior around this flow. (role: plugin install/update infrastructure contributor; confidence: high; commits: 97d1f5fd153c, 2eaf8ad7126b, 0946e37523f9; files: src/infra/install-source-utils.ts, src/plugins/install.ts, src/plugins/update.ts)
  • velvet-shark: Merged the current-main official plugin update change that touched the same updater files and led to this branch's conflict-resolution merge. (role: recent adjacent contributor; confidence: medium; commits: 0247eab77335; files: src/plugins/update.ts, src/plugins/update.test.ts, src/cli/plugins-update-command.ts)
  • BKF-Gitty: Recent merged history ties this handle to plugin channel fallback messaging and tests in the same update flow. (role: recent update fallback contributor; confidence: medium; commits: c96a12d3c887; files: src/plugins/update.ts, src/plugins/update.test.ts, src/cli/update-cli/update-command.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: 🧂 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. P2 Normal backlog priority with limited blast radius. labels Jun 24, 2026
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed 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 Jun 25, 2026
@RomneyDa RomneyDa self-assigned this Jun 25, 2026
@RomneyDa
RomneyDa merged commit dd0e4f6 into openclaw:main Jun 26, 2026
95 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 26, 2026
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
wheakerd pushed a commit to wheakerd/clawdbot that referenced this pull request Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S 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.

[Bug]: Plugin update emits spurious 'unsupported npm spec' error after transient npm view failure

2 participants