Skip to content

Commit fa4df7f

Browse files
committed
fix(storage): purge orphaned feature_set_members (migration 017)
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 <maa.ashik00@gmail.com>
1 parent 92f8ac2 commit fa4df7f

3 files changed

Lines changed: 86 additions & 0 deletions

File tree

crates/mcpmux-storage/src/database.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ const MIGRATIONS: &[Migration] = &[
113113
name: "space_builtin_servers",
114114
sql: include_str!("migrations/016_space_builtin_servers.sql"),
115115
},
116+
Migration {
117+
version: 17,
118+
name: "purge_orphaned_feature_set_members",
119+
sql: include_str!("migrations/017_purge_orphaned_feature_set_members.sql"),
120+
},
116121
];
117122

118123
/// SQLite database wrapper.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
-- Migration 017: Purge orphaned feature_set_members.
2+
--
3+
-- The refactor (#151) changed a feature member's identity from a qualified
4+
-- "server_id/tool_name" string to the `server_features.id` UUID, but no
5+
-- migration converted the rows that migration 001's `default` (now "Starter")
6+
-- set had accumulated under the old model. On upgraded installs those members
7+
-- survived as dangling rows whose `member_id` resolves to no live feature, so:
8+
-- * the FeatureSets card counted the raw rows (e.g. "93 members"), while
9+
-- * the detail/resolver matched against live feature ids and found none
10+
-- selected — the set effectively granted 0 tools.
11+
--
12+
-- Those orphaned members already resolve to nothing, so deleting them changes
13+
-- no effective behavior — it just makes the count honest and leaves the
14+
-- Starter set empty, identical to a freshly created Space (the new model gives
15+
-- the Starter set no special routing role). Composition members that point at
16+
-- a deleted set are cleaned up the same way.
17+
--
18+
-- Idempotent: re-running deletes nothing once the table is consistent. Any
19+
-- valid members a user added under the new model are kept (their member_id
20+
-- still resolves).
21+
22+
-- Feature members whose target feature no longer exists.
23+
DELETE FROM feature_set_members
24+
WHERE member_type = 'feature'
25+
AND member_id NOT IN (SELECT id FROM server_features);
26+
27+
-- Composition members whose target FeatureSet no longer exists.
28+
DELETE FROM feature_set_members
29+
WHERE member_type = 'feature_set'
30+
AND member_id NOT IN (SELECT id FROM feature_sets);

tests/rust/tests/database/migrations.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,54 @@ fn test_in_memory_database() {
4444
// Verify it's usable (we can't really check much else)
4545
drop(db);
4646
}
47+
48+
/// Migration 017 must drop feature_set_members that point at a feature or
49+
/// feature set that no longer exists (orphans from the pre-refactor member
50+
/// identity), while keeping members that still resolve.
51+
#[test]
52+
fn test_017_purges_orphaned_feature_set_members() {
53+
let test_db = TestDatabase::new();
54+
let conn = test_db.db.connection();
55+
56+
// Seed a space, a custom FS, one live feature, and three members:
57+
// m1 — valid feature member (points to the live feature) → keep
58+
// m2 — orphaned feature member (old "server/tool" identity) → purge
59+
// m3 — composition member pointing at a deleted FS → purge
60+
conn.execute_batch(
61+
"INSERT INTO spaces (id,name,icon,description,is_default,sort_order,created_at,updated_at)
62+
VALUES ('s1','S','x','',0,0,datetime('now'),datetime('now'));
63+
INSERT INTO feature_sets
64+
(id,name,description,icon,space_id,feature_set_type,server_id,is_builtin,is_deleted,created_at,updated_at)
65+
VALUES ('fs1','FS','','','s1','custom',NULL,0,0,datetime('now'),datetime('now'));
66+
INSERT INTO server_features
67+
(id,space_id,server_id,feature_type,feature_name,discovered_at,last_seen_at,is_available)
68+
VALUES ('feat-live','s1','srv','tool','do_thing',datetime('now'),datetime('now'),1);
69+
INSERT INTO feature_set_members (id,feature_set_id,member_type,member_id,mode,created_at) VALUES
70+
('m1','fs1','feature','feat-live','include',datetime('now')),
71+
('m2','fs1','feature','srv/do_thing','include',datetime('now')),
72+
('m3','fs1','feature_set','fs-gone','include',datetime('now'));",
73+
)
74+
.expect("seed failed");
75+
76+
// Re-apply migration 017 (idempotent) to exercise the purge on the seeded orphans.
77+
conn.execute_batch(include_str!(
78+
"../../../../crates/mcpmux-storage/src/migrations/017_purge_orphaned_feature_set_members.sql"
79+
))
80+
.expect("migration 017 failed");
81+
82+
let remaining: Vec<String> = {
83+
let mut stmt = conn
84+
.prepare("SELECT member_id FROM feature_set_members WHERE feature_set_id='fs1' ORDER BY member_id")
85+
.unwrap();
86+
stmt.query_map([], |r| r.get::<_, String>(0))
87+
.unwrap()
88+
.collect::<Result<_, _>>()
89+
.unwrap()
90+
};
91+
92+
assert_eq!(
93+
remaining,
94+
vec!["feat-live".to_string()],
95+
"only the still-resolvable feature member should survive the purge"
96+
);
97+
}

0 commit comments

Comments
 (0)