fix(state): track last-migrated version so plugin migrations surface on upgrade (#320)#421
Open
bitwize-music wants to merge 3 commits into
Open
fix(state): track last-migrated version so plugin migrations surface on upgrade (#320)#421bitwize-music wants to merge 3 commits into
bitwize-music wants to merge 3 commits into
Conversation
…on upgrade (#320) Step 4.5's migration check compared state's plugin_version against the manifest, but build_state/incremental_update overwrote plugin_version with the installed version on every rebuild — so "stored < current" was never true and migrations were silently skipped for every upgrading user. No code parsed or surfaced migrations either; Step 4.5 was prose with nothing behind it. Separate last_migrated_version (advances only on explicit acknowledgment, preserved across rebuilds) from plugin_version (installed version, display only). Add get_pending_migrations + acknowledge_migrations MCP tools backed by pure, tested helpers. A pre-tracking state (null) now surfaces the full backlog once instead of skipping; a fresh install is seeded to the installed version. No schema-version bump — bumping would force the live MCP path to auto-rebuild every state and erase the "behind" status the fix depends on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s reason Addresses self-review of #320: - Add integration tests proving StateCache.rebuild() carries last_migrated_version over the freshly-built installed seed (and records None for pre-field states). The pure helper was tested but its wiring into rebuild() was not — removing the carry line previously kept the suite green. - get_pending_migrations now returns reason "unknown" (not "current") when the installed version can't be read, so callers can distinguish "up to date" from "couldn't read plugin.json". - Hoist the per-call indexer import in health.py to module level. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…320) acknowledge_migrations computed `target = version or _read_plugin_version()` and wrote it unconditionally. With no version argument and an unreadable plugin.json, target was None, so it wrote last_migrated_version=None — which silently un-acknowledged the entire backlog and resurfaced it on the next session, while still reporting success. Now it returns an error and leaves state untouched when the version can't be determined. Also cover the previously-untested `"error" in state` short-circuit, and document the `reason: "unknown"` outcome (plugin.json unreadable -> empty pending -> no action) in CLAUDE.md Step 4.5 and the session-start skill. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #320 — plugin migration notes never surfaced on upgrade.
The session-start migration check (Step 4.5) compared state's
plugin_versionagainst the manifest, butbuild_state/incremental_updateoverwroteplugin_versionwith the installed version on every rebuild. So "stored < current" could never be true and migrations were silently skipped for every upgrading user. Worse, no code actually parsed or surfaced migrations — Step 4.5 was prose with nothing behind it.What changed
State model (
tools/state/indexer.py)last_migrated_version— the version through which migrations have been processed. Only advances on explicit acknowledgment; preserved across rebuilds.plugin_versionkeeps its meaning (installed version, for display).build_stateseedslast_migrated_versionto the installed version (correct for a brand-new install — nothing historical to surface).carry_migration_tracking()preserves the prior value across full rebuilds, so a rebuild never silently acknowledges pending migrations. Wired intocmd_rebuild(CLI),StateCache.rebuild()and the_load_from_diskauto-rebuild (MCP).parse_migration_file()andget_pending_migrations()compute the pending notes betweenlast_migrated_versionand the installed version, sorted ascending.validate_statetype-checks the new field (optional, so legacy states still validate).MCP tools (
handlers/health.py,server.py)get_pending_migrations— returns pending notes (parsed + sorted) with areason(current|upgrade|untracked).acknowledge_migrations— advanceslast_migrated_versionand persists (new thread-safeStateCache.acknowledge_migrations).get_plugin_version'sneeds_upgradenow reflects actual pending migrations instead of the (clobbered, unreliable) version diff.Null semantics. A pre-tracking state (
last_migrated_versionnull) now surfaces the full backlog once (reason: "untracked") instead of silently skipping — fixing the reporter's exact case. A fresh install is seeded to the installed version, so it sees nothing (no historical bombardment).No schema-version bump — deliberately. The live MCP path auto-rebuilds via
build_stateon aversionmismatch, which would erase the very "behind" status the fix depends on.last_migrated_versionis added as a backward-compatible optional field instead.Docs —
skills/session-start/SKILL.md(Step 4.5),CLAUDE.md,migrations/README.md,reference/state-schema.md,CHANGELOG.md.Testing
tests/unit/state/test_migration_tracking.py(24 tests): build_state seeding, incremental preservation,carry_migration_tracking,parse_migration_file,get_pending_migrations(including issue bug: plugin_version tracking makes migrations undetectable on upgrade #320's exact acceptance scenario),validate_state.get_pending_migrations/acknowledge_migrationstools, the full upgrade loop (behind → surfaced → acknowledged → clear), andStateCache.acknowledge_migrationspersistence. Updated twoget_plugin_versiontests to the corrected semantics.ruff,bandit,mypy(no issues), and the completepytestsuite.🤖 Generated with Claude Code