Skip to content

fix(state): retire superseded plugin install index#90474

Open
849261680 wants to merge 1 commit into
openclaw:mainfrom
849261680:fix/90418-plugin-index-conflict
Open

fix(state): retire superseded plugin install index#90474
849261680 wants to merge 1 commit into
openclaw:mainfrom
849261680:fix/90418-plugin-index-conflict

Conversation

@849261680

@849261680 849261680 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

What problem does this PR solve?

  • Fixes repeated legacy plugin install index conflict warnings after a successful external plugin update leaves newer shared SQLite install metadata for the same npm package.
  • Teaches the legacy install-index migration to treat an older legacy npm record as superseded only when the current SQLite record has the same npm package identity, a managed install path, and a strictly newer parsed version.
  • Keeps true conflicts fail-safe: same-version selector drift, package-name drift, malformed metadata, or newer legacy metadata still leaves the legacy source in place with the existing warning.

Why does this matter now?

Issue #90418 reports macOS upgrades from 2026.5.18 to 2026.6.1 repeatedly warning for codex and discord even though the plugin update completed and runtime health was good.

What is the intended outcome?

After openclaw doctor --fix or the migration path sees newer current SQLite install records for the same npm plugins, the stale legacy plugins/installs.json source is archived so normal doctor/status/restart flows do not keep reporting the same conflict.

What is intentionally out of scope?

  • No plugin API, config surface, or runtime fallback changes.
  • No broad reconciliation for same-version floating selector differences or malformed install metadata.
  • No changes to codex or discord plugin packages.

What does success look like?

A codex/discord state shaped like the reported upgrade archives the stale legacy install index once, preserves the newer SQLite records, and a second doctor run has no repeated plugin install index conflict warning.

What should reviewers focus on?

The safety boundary in legacyNpmInstallRecordSupersededByCurrent: same npm package identity, managed current install path, and strictly newer version are all required before archiving the legacy source.

Linked context

Which issue does this close?

Closes #90418

Which issues, PRs, or discussions are related?

Related #90418

Was this requested by a maintainer or owner?

No direct maintainer request; this follows the open bug report and ClawSweeper source-repro guidance on the issue.

Real behavior proof (required for external PRs)

  • Behavior addressed: repeated legacy plugin install index conflict warnings for codex/discord after current shared SQLite metadata already points at newer npm installs.
  • Real environment tested: local macOS source checkout with isolated HOME, OPENCLAW_CONFIG, and OPENCLAW_STATE_DIR; no user config or credentials used.
  • Exact steps or command run after this patch: seeded an isolated shared SQLite installed_plugin_index with codex/discord records at 2026.6.1, seeded legacy plugins/installs.json with codex/discord records at 2026.5.18, ran pnpm openclaw doctor --non-interactive --fix, then ran pnpm openclaw doctor --non-interactive again against the same isolated state.
  • Evidence after fix:
legacy_json_after_fix=archived
legacy_archive_after_fix=present
second_doctor_conflict_warning=absent

--- doctor --fix excerpt ---
13:│  - Archived plugin install index legacy source →                          │

--- second doctor warning check ---
no repeated plugin install index warning
  • Observed result after fix: the legacy install index source was archived on the fix run, and the follow-up doctor run did not emit conflicting plugin install metadata or Left plugin install index.
  • What was not tested: the full reported supervised update flow with Homebrew/global pnpm package swap and LaunchAgent restart.
  • Proof limitations or environment constraints: the live proof uses a synthetic isolated state matching the reported codex/discord metadata shape instead of a real 2026.5.18 to 2026.6.1 macOS upgrade. The local $autoreview and $agent-transcript skill files advertised to this Codex session were not present on disk, so I could not run those skill-specific workflows.
  • Before evidence (optional but encouraged): current issue evidence and the existing conflict branch show that a legacy plugins/installs.json source remains pending when SQLite has conflicting install metadata, causing repeated detection on later migration/doctor paths.

Tests and validation

Which commands did you run?

node scripts/run-vitest.mjs src/commands/doctor-state-migrations.test.ts
node scripts/run-vitest.mjs src/cli/update-cli/post-core-plugin-convergence.test.ts
./node_modules/.bin/oxfmt --check --threads=1 src/infra/state-migrations.ts src/commands/doctor-state-migrations.test.ts
git diff --check
node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json src/infra/state-migrations.ts src/commands/doctor-state-migrations.test.ts
pnpm openclaw doctor --non-interactive --fix
pnpm openclaw doctor --non-interactive

What regression coverage was added or updated?

Added focused migration tests for:

  • archiving older codex/discord legacy npm install records when current SQLite records are newer for the same package;
  • preserving the warning/source file when the current SQLite record is older than the legacy metadata.

What failed before this fix, if known?

Before this fix, the same current-vs-legacy codex/discord shape would be treated as conflicting metadata and leave plugins/installs.json in place, making future migration/doctor paths repeat the warning.

If no test was added, why not?

Tests were added.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. A stale legacy install-index warning is suppressed only after the migration archives an older superseded legacy npm record.

Did config, environment, or migration behavior change? (Yes/No)

Yes. Legacy state migration behavior changes for superseded npm plugin install records.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

Incorrectly archiving a legacy install index that still contains meaningful install metadata.

How is that risk mitigated?

The new branch requires same npm package identity, a current managed install path, and a strictly newer current version. Existing richer-match behavior and same-version conflict tests remain intact, and a new negative test covers the newer-legacy case.

Current review state

What is the next action?

Maintainer review and CI.

What is still waiting on author, maintainer, CI, or external proof?

No author-side code work is pending. Full supervised upgrade proof was not run locally; CI and maintainer judgment can decide whether that broader proof is needed.

Which bot or reviewer comments were addressed?

Addresses the ClawSweeper source-repro guidance on issue #90418 by targeting the legacy state/plugin install index migration layer and adding the requested regression coverage.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 4, 2026
@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 5, 2026, 3:15 AM ET / 07:15 UTC.

Summary
The PR adds a guarded npm install-record supersession check to legacy state migration and regression tests for archiving older codex/discord legacy records while preserving newer-legacy conflicts.

PR surface: Source +84, Tests +77. Total +161 across 2 files.

Reproducibility: yes. From source, current main and v2026.6.1 return before archiving when legacy/current install records conflict, so the legacy file remains and future doctor or migration runs can warn again; I did not run a local repro because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Legacy Migration Decision: 1 conflict branch widened. The changed branch controls whether existing upgrade state is archived or kept warning on later migration and doctor paths.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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

  • [P1] This changes upgrade-state migration semantics: a record that passes the supersession guard causes the legacy JSON source to be archived instead of preserved for repeated warnings.
  • [P1] The proof uses synthetic isolated codex/discord state rather than a full supervised 2026.5.18 to 2026.6.1 macOS upgrade flow.

Maintainer options:

  1. Accept Guarded Archival After Review (recommended)
    Maintainers can land the narrow older-record rule once they accept the synthetic proof level and the compatibility tradeoff for legacy install metadata.
  2. Require Full Upgrade Proof
    Maintainers can ask for a real 2026.5.18 to 2026.6.1 supervised macOS upgrade run before merge if the synthetic state proof is not enough.
  3. Defer To Broader Conflict Policy
    Maintainers can pause this PR if they want to settle the broader conflict-archival policy in fix(doctor): archive conflicting plugin install index #90267 first.

Next step before merge

  • [P2] Human maintainer review should decide whether the migration proof level and compatibility tradeoff are acceptable; no concrete automated repair is needed.

Security
Cleared: No concrete security or supply-chain issue found; the diff changes internal migration logic and focused tests without new dependencies, scripts, permissions, or secret handling.

Review details

Best possible solution:

Land the guarded superseded-record migration after maintainer acceptance of the upgrade risk, preserving fail-safe warnings for unproven conflicts.

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

Yes. From source, current main and v2026.6.1 return before archiving when legacy/current install records conflict, so the legacy file remains and future doctor or migration runs can warn again; I did not run a local repro because this review is read-only.

Is this the best way to solve the issue?

Yes, with maintainer acceptance of the compatibility tradeoff. This is the right layer and narrower than archiving every conflict because it keeps same-version selector drift, malformed metadata, and newer legacy metadata on the existing fail-safe warning path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from an isolated macOS source checkout showing the legacy JSON archived and a second doctor run without the repeated conflict warning.

Label justifications:

  • P2: This is a bounded upgrade-state bug with confusing repeated warnings but no reported runtime outage or data loss.
  • merge-risk: 🚨 compatibility: The PR changes legacy state migration behavior by archiving a previously preserved install-index file in some upgrade states.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 after-fix terminal output from an isolated macOS source checkout showing the legacy JSON archived and a second doctor run without the repeated conflict warning.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from an isolated macOS source checkout showing the legacy JSON archived and a second doctor run without the repeated conflict warning.
Evidence reviewed

PR surface:

Source +84, Tests +77. Total +161 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 84 0 +84
Tests 1 77 0 +77
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 161 0 +161

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/commands/doctor-state-migrations.test.ts.
  • [P1] node scripts/run-vitest.mjs src/cli/update-cli/post-core-plugin-convergence.test.ts.
  • [P1] git diff --check.

What I checked:

  • Repository policy read: Root AGENTS.md was read fully; no scoped AGENTS.md owns the touched src/infra or src/commands paths, so root migration, compatibility, and PR review guidance applies. (AGENTS.md:1, 1a3ce7c2a8da)
  • Current-main conflict path: Current main returns the repeated conflict warning before archiveLegacyInstalledPluginIndex, leaving plugins/installs.json available for later migration runs. (src/infra/state-migrations.ts:1501, 1a3ce7c2a8da)
  • Shipped behavior check: The same conflict-return behavior is present in v2026.6.1, matching the linked upgrade report's shipped warning path. (src/infra/state-migrations.ts:1500, 2e08f0f4221f)
  • PR implementation guard: The PR only treats a legacy npm record as superseded when both records are npm, package identity matches, current has installPath, and current version compares strictly newer. (src/infra/state-migrations.ts:463, eb31b1f2255f)
  • PR regression coverage: The PR adds a positive codex/discord older-legacy archival test and a negative test that keeps the legacy source when legacy metadata is newer than SQLite metadata. (src/commands/doctor-state-migrations.test.ts:1411, eb31b1f2255f)
  • Install-record contract: The npm update path persists source, spec, installPath, version, and npm resolution fields, matching the record fields this PR uses to identify newer current metadata. (src/plugins/update.ts:2035, 1a3ce7c2a8da)

Likely related people:

  • steipete: Current blame ties the legacy install-index comparator and conflict-return path to Peter Steinberger in the current checked-out history. (role: recent area contributor; confidence: high; commits: 82710b4f1f10; files: src/infra/state-migrations.ts, src/commands/doctor-state-migrations.test.ts)
  • Dallin Romney: Commit 30b9e12 changed the same state migration and doctor migration test files shortly before this PR's reported regression path. (role: recent adjacent contributor; confidence: medium; commits: 30b9e123b871; files: src/infra/state-migrations.ts, src/commands/doctor-state-migrations.test.ts)
  • vincentkoc: The affected latest release tag v2026.6.1 points to Vincent Koc's release commit, making him useful for shipped-upgrade routing even though the migration code history points elsewhere. (role: release and upgrade context; confidence: medium; commits: 2e08f0f4221f; files: src/infra/state-migrations.ts, src/cli/update-cli/update-command.ts, src/plugins/installed-plugin-index-store.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 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands Command implementations 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. proof: supplied External PR includes structured after-fix real behavior proof. 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]: Upgrade leaves repeated shared SQLite plugin install metadata conflict warnings for codex/discord

1 participant