fix(state): retire superseded plugin install index#90474
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 5, 2026, 3:15 AM ET / 07:15 UTC. Summary 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.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest 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 changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +84, Tests +77. Total +161 across 2 files. View PR surface stats
Acceptance criteria:
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
|
12a4595 to
eb31b1f
Compare
Summary
What problem does this PR solve?
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 --fixor the migration path sees newer current SQLite install records for the same npm plugins, the stale legacyplugins/installs.jsonsource is archived so normal doctor/status/restart flows do not keep reporting the same conflict.What is intentionally out of scope?
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)
HOME,OPENCLAW_CONFIG, andOPENCLAW_STATE_DIR; no user config or credentials used.installed_plugin_indexwith codex/discord records at2026.6.1, seeded legacyplugins/installs.jsonwith codex/discord records at2026.5.18, ranpnpm openclaw doctor --non-interactive --fix, then ranpnpm openclaw doctor --non-interactiveagain against the same isolated state.conflicting plugin install metadataorLeft plugin install index.$autoreviewand$agent-transcriptskill files advertised to this Codex session were not present on disk, so I could not run those skill-specific workflows.plugins/installs.jsonsource remains pending when SQLite has conflicting install metadata, causing repeated detection on later migration/doctor paths.Tests and validation
Which commands did you run?
What regression coverage was added or updated?
Added focused migration tests for:
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.jsonin 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.