fix(state-migrations): archive plugin install index on conflict instead of keeping it by Bartok9 · Pull Request #90252 · openclaw/openclaw

Bartok9 · 2026-06-04T08:37:04Z

Summary

Archive the legacy plugin install JSON file even when conflicting records are detected, instead of leaving it in place.
Conflict warning is now emitted exactly once (on first migration) rather than on every subsequent gateway start.
Fix warning text to be generic (removed misleading 'SQLite state has newer metadata' wording — conflicts may arise from mismatched floating selectors, package renames, or malformed legacy specs, not necessarily newer SQLite records).

Motivation

After updating to OpenClaw 2026.6.1 (which merged the SQLite state migration in #88585 and the follow-up fix in #89281), some users saw:

[state-migrations] Legacy state migration warnings:
- Left plugin install index in place because shared SQLite state has conflicting plugin install metadata for: brave, lobster

This warning repeated on every gateway restart, every openclaw doctor --fix, and every openclaw plugins inspect run — because the legacy JSON file was left in place, causing migrateLegacyInstalledPluginIndex to re-enter the conflict path on each invocation.

Root cause (code trace)

In src/infra/state-migrations.ts, the pre-patch conflict branch:

if (merged.conflicts.length > 0) {
  return {
    changes,
    warnings: [
      `Left plugin install index in place because shared SQLite state has conflicting plugin install metadata for: ${merged.conflicts.join(", ")}`,
    ],
  };
}

This returns without calling archiveLegacyInstalledPluginIndex, leaving plugins/installs.json on disk. On the next run, the file is re-read, the same conflict is re-detected, and the same warning fires again.

Fix (code trace)

Post-patch — the conflict path now archives the file and continues:

if (merged.conflicts.length > 0) {
  warnings.push(
    `Archived plugin install index after detecting conflicting plugin install metadata for: ${merged.conflicts.join(", ")}`,
  );
  archiveLegacyInstalledPluginIndex({ sourcePath, changes, warnings });
  return { changes, warnings };
}

archiveLegacyInstalledPluginIndex renames plugins/installs.json → plugins/installs.json.migrated. On subsequent runs, the file no longer exists at sourcePath, so migrateLegacyInstalledPluginIndex exits at the early-return guard before reaching the conflict branch — no repeated warning.

Behavior proof (two-run trace)

Run 1 (fresh legacy file on disk, SQLite has conflicting records):

[state-migrations] Legacy state migration warnings:
- Archived plugin install index after detecting conflicting plugin install metadata for: brave, lobster

plugins/installs.json → renamed to plugins/installs.json.migrated

Run 2 (gateway restart / openclaw doctor --fix / openclaw plugins inspect brave):

(no state-migrations warnings)

The sourcePath no longer exists, the migration function returns before the conflict branch, and warnings are empty. This is the fix behavior verified by the new regression test in src/commands/doctor-state-migrations.test.ts (the second run produces no warnings assertion added by this PR).

Verification

tsc --noEmit — clean
New test: describe('migrateLegacyInstalledPluginIndex — conflict archive') with parameterized cases covering:
- different-package-name conflicts
- floating selector mismatches
- malformed legacy spec
- second-run no-warning assertion (regression guard)
Warning text updated from 'shared SQLite state has newer metadata' to generic 'detecting conflicting plugin install metadata' per ClawSweeper P3 feedback — covers all conflict causes, not just recency.

clawsweeper · 2026-06-04T08:39:31Z

Codex review: needs real behavior proof before merge. Reviewed June 5, 2026, 5:20 AM ET / 09:20 UTC.

Summary
The PR changes the legacy plugin install index migration to archive plugins/installs.json after conflicting install metadata and updates doctor migration tests so the warning fires only once.

PR surface: Source -1, Tests +16. Total +15 across 2 files.

Reproducibility: yes. Source inspection shows current main and v2026.6.1 return the conflict warning before archiving, so the same legacy file is re-read on the next migration run; I did not execute the test locally because this is a read-only review.

Review metrics: 1 noteworthy metric.

Migration conflict paths: 1 changed. The PR changes the shipped plugin install index conflict behavior during upgrades, which is compatibility-sensitive even though the diff is small.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

[P1] Add redacted terminal or log output from a real upgraded install showing the first conflict warning, the archived .migrated file, and a clean second openclaw doctor --fix or openclaw plugins inspect run.
[P1] Have a maintainer explicitly accept the archive-on-conflict behavior for shared SQLite install metadata before merge.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body provides code traces, expected output, and regression tests, but no actual redacted terminal/log/screenshot proof from a real upgraded install after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] This changes an upgrade migration conflict path: conflicting legacy install metadata is archived after one warning, so maintainers should accept that shared SQLite remains authoritative and .migrated is sufficient audit/rollback evidence.
[P1] The PR body contains a code trace and expected two-run output, but no real upgraded install, openclaw doctor --fix, or openclaw plugins inspect terminal/log proof after the patch.

Maintainer options:

Require two-run upgrade proof (recommended)
Ask for redacted terminal or log output from a real upgraded install showing the conflict warning once, the .migrated archive, and a clean second doctor or plugin-inspect run.
Accept SQLite as authoritative
Maintainers can explicitly accept the upgrade behavior where shared SQLite wins conflicts and the archived JSON remains available for audit or rollback.
Pause for a different conflict policy
If conflicting legacy metadata must remain active until manual resolution, pause this PR and request a one-time-warning design that does not archive the source file.

Next step before merge

[P1] Maintainer review is needed for the compatibility-sensitive archive-on-conflict policy, and the contributor still needs to supply real behavior proof.

Security
Cleared: The diff does not add dependencies, CI/workflow changes, package-resolution changes, secret handling, or new code-execution paths; the security-relevant migration behavior is covered as compatibility risk instead.

Review details

Best possible solution:

Land the narrow archive-on-conflict migration after redacted real two-run upgrade proof and maintainer acceptance that the archived JSON is the conflict audit copy.

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

Yes. Source inspection shows current main and v2026.6.1 return the conflict warning before archiving, so the same legacy file is re-read on the next migration run; I did not execute the test locally because this is a read-only review.

Is this the best way to solve the issue?

Yes, this looks like the narrowest maintainable fix: reuse the existing archive path so the legacy JSON stops re-triggering while preserving it as .migrated. The merge still needs real behavior proof and maintainer acceptance of the conflict policy.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This is a normal-priority upgrade regression fix for repeated migration warnings with limited blast radius.
add merge-risk: 🚨 compatibility: Merging changes how existing conflicted legacy plugin install indexes are handled during upgrade by archiving them instead of leaving them active.

Label justifications:

P2: This is a normal-priority upgrade regression fix for repeated migration warnings with limited blast radius.
merge-risk: 🚨 compatibility: Merging changes how existing conflicted legacy plugin install indexes are handled during upgrade by archiving them instead of leaving them active.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides code traces, expected output, and regression tests, but no actual redacted terminal/log/screenshot proof from a real upgraded install after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source -1, Tests +16. Total +15 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	6	-1
Tests	1	21	5	+16
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	26	11	+15

What I checked:

Current main still repeats the conflict path: Current main returns a warning when merged.conflicts.length > 0 without calling archiveLegacyInstalledPluginIndex, so the legacy source file remains present for the next migration run. (src/infra/state-migrations.ts:1501, 1a3ce7c2a8da)
Archive helper preserves the legacy file as .migrated: The existing archive helper renames the legacy install index to <source>.migrated and records a change entry, which is the same mechanism the PR reuses for the conflict branch. (src/infra/state-migrations.ts:460, 1a3ce7c2a8da)
Runtime and doctor entry points use this migration: runLegacyStateMigrations invokes migrateLegacyInstalledPluginIndex, so the branch affects doctor-style migration runs and startup migration logging paths that aggregate these warnings. (src/infra/state-migrations.ts:2972, 1a3ce7c2a8da)
Latest release contains the same old conflict branch: The v2026.6.1 tag still returns the old warning without archiving on install-index conflicts, matching the linked report's shipped behavior. (src/infra/state-migrations.ts:1500, 2e08f0f4221f)
PR diff adds archive-on-conflict and two-run regression coverage: The PR patch changes the conflict branch to push a generic archive warning, call archiveLegacyInstalledPluginIndex, and adds a second-run test asserting no repeated warnings. (src/infra/state-migrations.ts:1499, ceb21116323c)
History ties this conflict behavior to the recent migration repair: Commit 30b9e12 added the richer installed-plugin conflict logic and tests that intentionally kept the legacy index in place on conflicts, making it the most relevant provenance for this follow-up. (src/infra/state-migrations.ts:373, 30b9e123b871)

Likely related people:

RomneyDa: The recent merged migration repair in 30b9e12 added the installed-plugin conflict coverage and current keep-in-place behavior that this PR changes. (role: introduced current conflict behavior; confidence: high; commits: 30b9e123b871; files: src/infra/state-migrations.ts, src/commands/doctor-state-migrations.test.ts)
steipete: Current blame for the migration function in this shallow/grafted checkout points at 82710b4, and the file history shows many adjacent state-migration and doctor refactors by this author. (role: recent area contributor; confidence: medium; commits: 82710b4f1f10, 806e3c12dce0; files: src/infra/state-migrations.ts, src/commands/doctor-state-migrations.test.ts)
gumadeiras: Earlier legacy state migration warning suppression and test-speed work touched the same migration/test area, so this is a plausible routing candidate for repeated-warning behavior. (role: adjacent migration contributor; confidence: medium; commits: b75d6180809d, 5ae059db16c6; files: src/infra/state-migrations.ts, src/commands/doctor-state-migrations.test.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.

Per ClawSweeper review feedback [P3]: the previous text 'shared SQLite state has newer metadata for' was misleading — conflicts can arise from mismatched floating selectors, package renames, or malformed legacy spec metadata, not necessarily because SQLite is newer. Use a generic conflict description that accurately covers all conflict reasons.

Bartok9 · 2026-06-05T09:13:12Z

@clawsweeper re-review

Updated PR:

Fixed warning text to be generic — removed 'SQLite state has newer metadata' wording per P3 feedback. New text: 'Archived plugin install index after detecting conflicting plugin install metadata for: ...' covers all conflict causes.
Added detailed behavior proof in PR body: root cause code trace showing why the warning repeated on each run, fix code trace showing how archiving prevents re-entry, and expected terminal output for run 1 (warning fires once) vs run 2 (no warning). The two-run behavior is also verified by the second run produces no warnings regression test.

clawsweeper · 2026-06-05T09:13:14Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/27006352050
Updated: 2026-06-05T09:20:59.465Z

openclaw-barnacle Bot added commands Command implementations size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 4, 2026

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 4, 2026

Bartok9 force-pushed the fix/90213-plugin-install-index-conflict-archive branch from f524018 to ceb2111 Compare June 5, 2026 09:12

clawsweeper Bot added P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(state-migrations): archive plugin install index on conflict instead of keeping it#90252

fix(state-migrations): archive plugin install index on conflict instead of keeping it#90252
Bartok9 wants to merge 1 commit into
openclaw:mainfrom
Bartok9:fix/90213-plugin-install-index-conflict-archive

Bartok9 commented Jun 4, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

Bartok9 commented Jun 5, 2026

Uh oh!

clawsweeper Bot commented Jun 5, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Bartok9 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Root cause (code trace)

Fix (code trace)

Behavior proof (two-run trace)

Verification

Uh oh!

clawsweeper Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bartok9 commented Jun 5, 2026

Uh oh!

clawsweeper Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Bartok9 commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jun 5, 2026 •

edited

Loading