Skip to content

Commit 6a46a5b

Browse files
committed
fix(featureset): lock the Starter's name (members-only editable)
Per review: the Starter shouldn't be renamable — only which tools/features it includes should be editable. It's the default fallback for unmapped folders, so a stable identity matters. - storage `update()` now preserves a builtin row's name/description/icon (the DB's own `is_builtin` governs, not the caller's struct) while still applying MEMBER changes. The member editor routes through `update()`, so editing the Starter's tools keeps working, but no caller can rename it. Custom sets are unaffected. New test: `test_update_locks_builtin_identity_but_allows_member_edits`. - Copy updated everywhere from "edit, rename, or delete freely" to "edit which tools it includes; its name is fixed and it can't be deleted": domain module + FeatureSetType doc + new_starter description, the space_repository seed, migration 018's target text (+ its test constant), the FeatureSets card/panel, and featureSets.ts. Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 8554f52 commit 6a46a5b

9 files changed

Lines changed: 96 additions & 30 deletions

File tree

apps/desktop/src/features/featuresets/FeatureSetPanel.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,11 @@ export function FeatureSetPanel({ featureSet, spaceId, onClose, onDelete, onUpda
5858

5959
// Both FS types are member-driven now.
6060
const isConfigurable = true;
61-
// The auto-seeded "Starter" FS is fully editable like a Custom one (edit /
62-
// rename / empty its members). The difference is routing + lifecycle: it's
63-
// the default fallback for unmapped folders and can't be deleted — so the
64-
// Delete action below is gated to Custom sets only.
61+
// The auto-seeded "Starter" FS has editable membership like a Custom one
62+
// (change which tools it includes, or empty it). What's locked is its
63+
// identity + lifecycle: it's the default fallback for unmapped folders, so
64+
// its name is fixed (the backend ignores name changes on builtin rows) and
65+
// it can't be deleted — the Delete action below is gated to Custom sets.
6566
const isStarter = isStarterFeatureSet(featureSet);
6667
const isCustom = featureSet.feature_set_type === 'custom';
6768

@@ -259,7 +260,7 @@ export function FeatureSetPanel({ featureSet, spaceId, onClose, onDelete, onUpda
259260
<span
260261
title={
261262
isStarter
262-
? "Auto-created with this Space. The default set for folders you haven't mapped — editable and renamable, but can't be deleted."
263+
? "Auto-created with this Space. The default set for folders you haven't mapped — edit which tools it includes; its name is fixed and it can't be deleted."
263264
: undefined
264265
}
265266
className={`text-[10px] px-1.5 py-0.5 rounded-full font-medium border ${
@@ -341,7 +342,7 @@ export function FeatureSetPanel({ featureSet, spaceId, onClose, onDelete, onUpda
341342
<div className="flex gap-2">
342343
<Star className="h-4 w-4 text-yellow-500 flex-shrink-0 mt-0.5" />
343344
<div className="text-xs text-yellow-800 dark:text-yellow-200">
344-
<strong>Starter FeatureSet:</strong> auto-created with this Space and used as the <em>default</em> for folders you haven&apos;t explicitly mapped (and rootless sessions). Edit, rename, or empty it to change what they get — but it <strong>can&apos;t be deleted</strong>, since the fallback always needs a target.
345+
<strong>Starter FeatureSet:</strong> auto-created with this Space and used as the <em>default</em> for folders you haven&apos;t explicitly mapped (and rootless sessions). Edit which tools it includes (or empty it) to change what they get. Its name is fixed and it <strong>can&apos;t be deleted</strong>, since the fallback always needs a stable target.
345346
</div>
346347
</div>
347348
</div>

apps/desktop/src/features/featuresets/FeatureSetsPage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ export function FeatureSetsPage() {
375375
{isStarter && (
376376
<div
377377
className="absolute right-3 top-3 flex items-center gap-1.5 rounded-full bg-gradient-to-r from-emerald-500 to-green-500 px-2.5 py-1 text-[10px] font-bold uppercase tracking-wider text-white shadow-[0_4px_12px_-2px_rgb(16_185_129/0.5)]"
378-
title="Auto-seeded with this Space. The default set for folders you haven't mapped — edit or rename it to change what they get. It can't be deleted."
378+
title="Auto-seeded with this Space. The default set for folders you haven't mapped — edit which tools it includes to change what they get. Its name is fixed and it can't be deleted."
379379
data-testid={`featureset-starter-badge-${fs.id}`}
380380
>
381381
<CheckCircle2 className="h-3 w-3" />

apps/desktop/src/lib/api/featureSets.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import { invoke } from '@tauri-apps/api/core';
99
/**
1010
* `starter` is the auto-seeded FS that comes with each Space. It's the
1111
* default fallback the resolver routes unmapped folders / rootless sessions
12-
* to, so it's builtin and can't be deleted (it can be edited, renamed, or
13-
* emptied). The legacy `'default'` value is accepted on read because
14-
* migration 013 rewrites stored rows lazily and a stale fetch could still
15-
* surface it; new writes use `'starter'`.
12+
* to, so it's builtin: its members are editable (change which tools it
13+
* includes, or empty it), but it can't be renamed or deleted. The legacy
14+
* `'default'` value is accepted on read because migration 013 rewrites stored
15+
* rows lazily and a stale fetch could still surface it; new writes use
16+
* `'starter'`.
1617
*/
1718
export type FeatureSetType = 'starter' | 'default' | 'custom';
1819

crates/mcpmux-core/src/domain/feature_set.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
//! Each FeatureSet is scoped to a space and is one of two types:
44
//! - **Starter**: auto-created with the Space. It's the **default fallback**
55
//! for folders that aren't explicitly mapped (and for rootless/unknown
6-
//! sessions) — the resolver routes them here instead of denying. Editable
7-
//! and renamable (empty it to grant nothing by default), but **builtin and
8-
//! not deletable** since the fallback always needs a target. (Pre-resolver-v3
9-
//! it was the "Default" type and also acted as the implicit fallback; that
10-
//! role is back after a stint where it was a no-op seed.)
6+
//! sessions) — the resolver routes them here instead of denying. Its
7+
//! membership is editable (change which tools it includes, or empty it to
8+
//! grant nothing by default), but its **identity is locked**: builtin, so
9+
//! not renamable and not deletable since the fallback always needs a stable
10+
//! target. (Pre-resolver-v3 it was the "Default" type and also acted as the
11+
//! implicit fallback; that role is back after a stint where it was a no-op
12+
//! seed.)
1113
//! - **Custom**: any other operator-defined FeatureSet.
1214
1315
use chrono::{DateTime, Utc};
@@ -25,9 +27,10 @@ use uuid::Uuid;
2527
#[derive(Default)]
2628
pub enum FeatureSetType {
2729
/// Auto-created with the Space and used as the **default fallback** for
28-
/// unmapped folders / rootless sessions. Editable and renamable, but
29-
/// builtin — not deletable. Was historically called `Default` (DB column
30-
/// value carried over via migration 013).
30+
/// unmapped folders / rootless sessions. Its members are editable, but its
31+
/// identity is locked: builtin — not renamable and not deletable. Was
32+
/// historically called `Default` (DB column value carried over via
33+
/// migration 013).
3134
Starter,
3235
/// Any operator-defined FeatureSet.
3336
#[default]
@@ -240,10 +243,10 @@ impl FeatureSet {
240243
id: format!("fs_default_{}", space_id),
241244
name: "Starter".to_string(),
242245
description: Some(
243-
"Auto-created with this Space. Unmapped folders fall back to \
244-
this set — it's the default toolset for anything you haven't \
245-
explicitly mapped. Edit or rename it to change what they get; \
246-
it can't be deleted."
246+
"Auto-created with this Space — the default set for folders \
247+
you haven't explicitly mapped. Edit which tools it includes \
248+
to change what they get. Its name is fixed and it can't be \
249+
deleted."
247250
.to_string(),
248251
),
249252
icon: Some("⭐".to_string()),

crates/mcpmux-storage/src/migrations/018_starter_is_default_fallback_copy.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
-- an operator who customized the copy keeps their change.
1313

1414
UPDATE feature_sets
15-
SET description = 'Auto-created with this Space. Unmapped folders fall back to this set — it''s the default toolset for anything you haven''t explicitly mapped. Edit or rename it to change what they get; it can''t be deleted.'
15+
SET description = 'Auto-created with this Spacethe default set for folders you haven''t explicitly mapped. Edit which tools it includes to change what they get. Its name is fixed and it can''t be deleted.'
1616
WHERE is_builtin = 1
1717
AND feature_set_type IN ('starter', 'default')
1818
AND description = 'Auto-created with this Space. Edit, rename, or delete freely — bindings and per-client grants pick FeatureSets explicitly, so this one has no special routing role.';

crates/mcpmux-storage/src/repositories/feature_set_repository.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,20 @@ impl FeatureSetRepository for SqliteFeatureSetRepository {
230230
let db = self.db.lock().await;
231231
let conn = db.connection();
232232

233+
// Builtin FeatureSets (the auto-seeded Starter) are the default
234+
// fallback for unmapped folders, so their identity is fixed: name,
235+
// description, and icon are preserved here regardless of the incoming
236+
// values — only the MEMBERS (replaced below) and `updated_at` are
237+
// editable. The DB's own `is_builtin` flag governs (not the caller's
238+
// struct), so the lock holds for every caller, including the
239+
// member-set command that routes through update(). Custom sets update
240+
// normally.
233241
let rows_affected = conn.execute(
234-
"UPDATE feature_sets
235-
SET name = ?2, description = ?3, icon = ?4, updated_at = ?5
242+
"UPDATE feature_sets
243+
SET name = CASE WHEN is_builtin = 1 THEN name ELSE ?2 END,
244+
description = CASE WHEN is_builtin = 1 THEN description ELSE ?3 END,
245+
icon = CASE WHEN is_builtin = 1 THEN icon ELSE ?4 END,
246+
updated_at = ?5
236247
WHERE id = ?1 AND is_deleted = 0",
237248
params![
238249
feature_set.id,

crates/mcpmux-storage/src/repositories/space_repository.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,11 @@ impl SpaceRepository for SqliteSpaceRepository {
111111
// is preserved for FK-stability across the rename (migration 013).
112112
// The Starter is the default fallback for folders that aren't
113113
// explicitly mapped (and rootless/unknown sessions), so it's
114-
// load-bearing and builtin (can't be deleted).
114+
// load-bearing and builtin: members are editable, but it can't be
115+
// renamed or deleted.
115116
conn.execute(
116117
"INSERT OR IGNORE INTO feature_sets (id, name, description, icon, space_id, feature_set_type, is_builtin, created_at, updated_at)
117-
VALUES (?1, 'Starter', 'Auto-created with this Space. Unmapped folders fall back to this set — it''s the default toolset for anything you haven''t explicitly mapped. Edit or rename it to change what they get; it can''t be deleted.', '⭐', ?2, 'starter', 1, ?3, ?3)",
118+
VALUES (?1, 'Starter', 'Auto-created with this Spacethe default set for folders you haven''t explicitly mapped. Edit which tools it includes to change what they get. Its name is fixed and it can''t be deleted.', '⭐', ?2, 'starter', 1, ?3, ?3)",
118119
params![
119120
format!("fs_default_{}", space_id),
120121
space_id,

tests/rust/tests/database/feature_set.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! Tests for feature set CRUD, builtin types (All, Default, ServerAll),
44
//! and feature member composition.
55
6-
use mcpmux_core::domain::{FeatureSetType, MemberMode};
6+
use mcpmux_core::domain::{FeatureSetMember, FeatureSetType, MemberMode};
77
use mcpmux_core::repository::{FeatureSetRepository, SpaceRepository};
88
use mcpmux_storage::{SqliteFeatureSetRepository, SqliteSpaceRepository};
99
use std::sync::Arc;
@@ -112,6 +112,55 @@ async fn test_update_feature_set() {
112112
assert_eq!(loaded.description, Some("New description".to_string()));
113113
}
114114

115+
/// The builtin Starter's identity is locked: `update()` must ignore name /
116+
/// description / icon changes on a builtin row (it's the default fallback, not
117+
/// renamable) while still applying MEMBER changes — that's how the member
118+
/// editor (which routes through `update()`) keeps working for the Starter.
119+
#[tokio::test]
120+
async fn test_update_locks_builtin_identity_but_allows_member_edits() {
121+
let test_db = TestDatabase::new();
122+
let db = Arc::new(Mutex::new(test_db.db));
123+
let feature_repo = SqliteFeatureSetRepository::new(Arc::clone(&db));
124+
let space_repo = SqliteSpaceRepository::new(db);
125+
126+
let space = fixtures::test_space("Test Space");
127+
SpaceRepository::create(&space_repo, &space).await.unwrap();
128+
129+
// Auto-seed the builtin Starter for this Space.
130+
FeatureSetRepository::ensure_builtin_for_space(&feature_repo, &space.id.to_string())
131+
.await
132+
.unwrap();
133+
let mut starter =
134+
FeatureSetRepository::get_starter_for_space(&feature_repo, &space.id.to_string())
135+
.await
136+
.unwrap()
137+
.expect("starter seeded");
138+
let original_name = starter.name.clone();
139+
let original_desc = starter.description.clone();
140+
141+
// Attempt to rename + re-describe the builtin AND add a member at once.
142+
starter.name = "Hacked Name".to_string();
143+
starter.description = Some("hacked description".to_string());
144+
starter.members = vec![FeatureSetMember::include_feature(&starter.id, "feat-x")];
145+
FeatureSetRepository::update(&feature_repo, &starter)
146+
.await
147+
.expect("update should succeed");
148+
149+
let loaded = FeatureSetRepository::get_with_members(&feature_repo, &starter.id)
150+
.await
151+
.unwrap()
152+
.unwrap();
153+
// Identity preserved...
154+
assert_eq!(loaded.name, original_name, "builtin name must not change");
155+
assert_eq!(
156+
loaded.description, original_desc,
157+
"builtin description must not change"
158+
);
159+
// ...but the member edit went through.
160+
assert_eq!(loaded.members.len(), 1, "members must stay editable");
161+
assert_eq!(loaded.members[0].member_id, "feat-x");
162+
}
163+
115164
#[tokio::test]
116165
async fn test_delete_feature_set() {
117166
let test_db = TestDatabase::new();

tests/rust/tests/database/migrations.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ fn test_017_purges_orphaned_feature_set_members() {
103103
#[test]
104104
fn test_018_rewrites_stale_starter_description_only() {
105105
const STALE: &str = "Auto-created with this Space. Edit, rename, or delete freely — bindings and per-client grants pick FeatureSets explicitly, so this one has no special routing role.";
106-
const FRESH: &str = "Auto-created with this Space. Unmapped folders fall back to this set — it's the default toolset for anything you haven't explicitly mapped. Edit or rename it to change what they get; it can't be deleted.";
106+
const FRESH: &str = "Auto-created with this Spacethe default set for folders you haven't explicitly mapped. Edit which tools it includes to change what they get. Its name is fixed and it can't be deleted.";
107107

108108
let test_db = TestDatabase::new();
109109
let conn = test_db.db.connection();

0 commit comments

Comments
 (0)