From fa4df7f65e221118f39ee2f7da78bc387dc2ed75 Mon Sep 17 00:00:00 2001 From: Mohammod Al Amin Ashik Date: Mon, 15 Jun 2026 16:39:07 +0800 Subject: [PATCH] fix(storage): purge orphaned feature_set_members (migration 017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #151 refactor changed a feature member's identity from a qualified "server_id/tool_name" string to the server_features.id UUID, but no migration converted the rows migration 001's default ("Starter") set had accumulated. On upgraded installs those members survived as orphans whose member_id resolves to no live feature, so the FeatureSets card counted the raw rows (e.g. "93 members") while the detail/resolver matched against live feature ids and found none selected — the set effectively granted 0 tools. Migration 017 deletes feature_set_members that point at a feature or feature set that no longer exists. Those members already resolve to nothing, so this changes no effective behavior — it makes the count honest and leaves the Starter set empty, identical to a freshly created Space (the new model gives Starter no special routing role; unmapped sessions deny + expose @mux meta-tools regardless). Idempotent; valid members kept. Adds a migration test covering keep-valid / purge-orphan. Signed-off-by: Mohammod Al Amin Ashik --- crates/mcpmux-storage/src/database.rs | 5 ++ ...017_purge_orphaned_feature_set_members.sql | 30 +++++++++++ tests/rust/tests/database/migrations.rs | 51 +++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 crates/mcpmux-storage/src/migrations/017_purge_orphaned_feature_set_members.sql diff --git a/crates/mcpmux-storage/src/database.rs b/crates/mcpmux-storage/src/database.rs index 8a19f12b..ca3b9271 100644 --- a/crates/mcpmux-storage/src/database.rs +++ b/crates/mcpmux-storage/src/database.rs @@ -113,6 +113,11 @@ const MIGRATIONS: &[Migration] = &[ name: "space_builtin_servers", sql: include_str!("migrations/016_space_builtin_servers.sql"), }, + Migration { + version: 17, + name: "purge_orphaned_feature_set_members", + sql: include_str!("migrations/017_purge_orphaned_feature_set_members.sql"), + }, ]; /// SQLite database wrapper. diff --git a/crates/mcpmux-storage/src/migrations/017_purge_orphaned_feature_set_members.sql b/crates/mcpmux-storage/src/migrations/017_purge_orphaned_feature_set_members.sql new file mode 100644 index 00000000..3aae4d01 --- /dev/null +++ b/crates/mcpmux-storage/src/migrations/017_purge_orphaned_feature_set_members.sql @@ -0,0 +1,30 @@ +-- Migration 017: Purge orphaned feature_set_members. +-- +-- The refactor (#151) changed a feature member's identity from a qualified +-- "server_id/tool_name" string to the `server_features.id` UUID, but no +-- migration converted the rows that migration 001's `default` (now "Starter") +-- set had accumulated under the old model. On upgraded installs those members +-- survived as dangling rows whose `member_id` resolves to no live feature, so: +-- * the FeatureSets card counted the raw rows (e.g. "93 members"), while +-- * the detail/resolver matched against live feature ids and found none +-- selected — the set effectively granted 0 tools. +-- +-- Those orphaned members already resolve to nothing, so deleting them changes +-- no effective behavior — it just makes the count honest and leaves the +-- Starter set empty, identical to a freshly created Space (the new model gives +-- the Starter set no special routing role). Composition members that point at +-- a deleted set are cleaned up the same way. +-- +-- Idempotent: re-running deletes nothing once the table is consistent. Any +-- valid members a user added under the new model are kept (their member_id +-- still resolves). + +-- Feature members whose target feature no longer exists. +DELETE FROM feature_set_members + WHERE member_type = 'feature' + AND member_id NOT IN (SELECT id FROM server_features); + +-- Composition members whose target FeatureSet no longer exists. +DELETE FROM feature_set_members + WHERE member_type = 'feature_set' + AND member_id NOT IN (SELECT id FROM feature_sets); diff --git a/tests/rust/tests/database/migrations.rs b/tests/rust/tests/database/migrations.rs index 831c64d2..747a526a 100644 --- a/tests/rust/tests/database/migrations.rs +++ b/tests/rust/tests/database/migrations.rs @@ -44,3 +44,54 @@ fn test_in_memory_database() { // Verify it's usable (we can't really check much else) drop(db); } + +/// Migration 017 must drop feature_set_members that point at a feature or +/// feature set that no longer exists (orphans from the pre-refactor member +/// identity), while keeping members that still resolve. +#[test] +fn test_017_purges_orphaned_feature_set_members() { + let test_db = TestDatabase::new(); + let conn = test_db.db.connection(); + + // Seed a space, a custom FS, one live feature, and three members: + // m1 — valid feature member (points to the live feature) → keep + // m2 — orphaned feature member (old "server/tool" identity) → purge + // m3 — composition member pointing at a deleted FS → purge + conn.execute_batch( + "INSERT INTO spaces (id,name,icon,description,is_default,sort_order,created_at,updated_at) + VALUES ('s1','S','x','',0,0,datetime('now'),datetime('now')); + INSERT INTO feature_sets + (id,name,description,icon,space_id,feature_set_type,server_id,is_builtin,is_deleted,created_at,updated_at) + VALUES ('fs1','FS','','','s1','custom',NULL,0,0,datetime('now'),datetime('now')); + INSERT INTO server_features + (id,space_id,server_id,feature_type,feature_name,discovered_at,last_seen_at,is_available) + VALUES ('feat-live','s1','srv','tool','do_thing',datetime('now'),datetime('now'),1); + INSERT INTO feature_set_members (id,feature_set_id,member_type,member_id,mode,created_at) VALUES + ('m1','fs1','feature','feat-live','include',datetime('now')), + ('m2','fs1','feature','srv/do_thing','include',datetime('now')), + ('m3','fs1','feature_set','fs-gone','include',datetime('now'));", + ) + .expect("seed failed"); + + // Re-apply migration 017 (idempotent) to exercise the purge on the seeded orphans. + conn.execute_batch(include_str!( + "../../../../crates/mcpmux-storage/src/migrations/017_purge_orphaned_feature_set_members.sql" + )) + .expect("migration 017 failed"); + + let remaining: Vec = { + let mut stmt = conn + .prepare("SELECT member_id FROM feature_set_members WHERE feature_set_id='fs1' ORDER BY member_id") + .unwrap(); + stmt.query_map([], |r| r.get::<_, String>(0)) + .unwrap() + .collect::>() + .unwrap() + }; + + assert_eq!( + remaining, + vec!["feat-live".to_string()], + "only the still-resolvable feature member should survive the purge" + ); +}