Skip to content

Commit 8554f52

Browse files
committed
feat(featureset): protect Starter from deletion + clarify mapping popup
Since the "default FeatureSet for unmapped roots" change, the Starter FS is the default fallback every unmapped folder routes to — so it's load-bearing and must not be deleted. Deletion was already blocked at the storage layer (builtin guard) and hidden in the panel (Custom-only delete); this aligns all the copy with that reality and makes the mapping popup non-confusing. - Starter is editable/renamable but NOT deletable. Replaced the now-false "no special routing role / delete freely" copy everywhere: domain module + FeatureSetType doc + new_starter description, the space_repository raw seed, the FeatureSets card/panel explainer, and featureSets.ts. - Migration 018 rewrites the stale Starter description on existing installs — only rows still matching the exact 014/015 seed text, so a customized copy is preserved (mirrors the 014/015 safety guard). - Mapping popup: title now states "This folder is using your Starter set" (configured now); buttons are "Modify" / "Close" instead of "Not now", which wrongly implied the folder was unmapped / had no tools. Tests: migration 018 rewrite-if-unchanged + preserve-customized. All green. Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 7fc50a0 commit 8554f52

9 files changed

Lines changed: 139 additions & 48 deletions

File tree

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ 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 treated identically to a Custom one
62-
// — the type tag is a UI hint, not a routing flag.
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.
6365
const isStarter = isStarterFeatureSet(featureSet);
6466
const isCustom = featureSet.feature_set_type === 'custom';
6567

@@ -257,7 +259,7 @@ export function FeatureSetPanel({ featureSet, spaceId, onClose, onDelete, onUpda
257259
<span
258260
title={
259261
isStarter
260-
? 'Auto-created with this Space. Edit, rename, or delete freely — no special routing role.'
262+
? "Auto-created with this Space. The default set for folders you haven't mapped — editable and renamable, but can't be deleted."
261263
: undefined
262264
}
263265
className={`text-[10px] px-1.5 py-0.5 rounded-full font-medium border ${
@@ -339,7 +341,7 @@ export function FeatureSetPanel({ featureSet, spaceId, onClose, onDelete, onUpda
339341
<div className="flex gap-2">
340342
<Star className="h-4 w-4 text-yellow-500 flex-shrink-0 mt-0.5" />
341343
<div className="text-xs text-yellow-800 dark:text-yellow-200">
342-
<strong>Starter FeatureSet:</strong> auto-created with this Space. It&apos;s an ordinary FeatureSet — edit, rename, or delete it freely. <em>No special routing role:</em> Workspace bindings and per-client grants pick FeatureSets explicitly.
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.
343345
</div>
344346
</div>
345347
</div>

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,9 @@ export function FeatureSetsPage() {
199199
})
200200
.sort((a, b) => {
201201
// Starter FS first (pinned to top — operator usually wants the
202-
// auto-seeded one near the top so they can edit / delete it
203-
// first), then Custom sets alphabetically. The 'default' key is
204-
// kept so a stale row read pre-migration still sorts correctly.
202+
// auto-seeded default set near the top so they can edit it first),
203+
// then Custom sets alphabetically. The 'default' key is kept so a
204+
// stale row read pre-migration still sorts correctly.
205205
const order: Record<string, number> = {
206206
starter: 0,
207207
default: 0,
@@ -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. Edit, rename, or delete freely — no special routing role; bindings and per-client grants pick FeatureSets explicitly."
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."
379379
data-testid={`featureset-starter-badge-${fs.id}`}
380380
>
381381
<CheckCircle2 className="h-3 w-3" />

apps/desktop/src/features/workspaces/WorkspaceBindingSheet.tsx

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
/**
22
* Workspace Binding Sheet
33
*
4-
* Fires when a connected client session resolves via source=Default for a
5-
* workspace root that has no binding yet. The user picks a Space + a
6-
* FeatureSet in that space, and we write a WorkspaceBinding locking both.
4+
* Fires when a connected client session reports a workspace root that has no
5+
* explicit binding yet — the folder is already working via the default
6+
* Starter set, and this sheet offers to map it to something else. The user
7+
* picks a Space + a FeatureSet, and we write a WorkspaceBinding locking both.
78
*
89
* • Space picker — defaults to the caller's current space, can be changed.
9-
* • FS picker — always includes a "space default" option (follow
10-
* whichever FS is active for the selected Space) plus
11-
* every Default + Custom set in that space.
12-
* • Dismiss — nothing written, ask again next session.
10+
* • FS picker — pre-selects the Space's Starter (the active default),
11+
* plus every Starter + Custom set in that space.
12+
* • Modify — writes the binding for the picked Space + FS.
13+
* • Close — nothing written; the folder keeps the default Starter
14+
* set, and the sheet re-offers next session.
1315
*
1416
* Committing the binding emits `WorkspaceBindingChanged` on the backend,
1517
* which triggers `notifications/tools/list_changed` — the client re-fetches
@@ -61,8 +63,8 @@ export function WorkspaceBindingSheet() {
6163
// Only dedupe the currently-open sheet against itself — if one is already
6264
// showing, swallow a second emit for the same session. We deliberately
6365
// don't dedupe across sessions / reconnects: the backend only emits when
64-
// `source=Default` (i.e. no binding exists), and reconnecting a client
65-
// is a normal signal that the user may want to configure the folder.
66+
// the folder has no explicit binding (it's on the default Starter set), and
67+
// reconnecting a client is a normal signal that the user may want to map it.
6668
// Persisting the dismissal in a ref would black-hole later attempts
6769
// until the next app restart, which is how this bug surfaced before.
6870
const currentSessionRef = useRef<string | null>(null);
@@ -74,7 +76,7 @@ export function WorkspaceBindingSheet() {
7476
(event) => {
7577
// Swallow only while a sheet is already showing — the user is
7678
// mid-decision, a second emit would stack a new sheet on top. Once
77-
// the current sheet closes (Save or Not now), the next emit from
79+
// the current sheet closes (Modify or Close), the next emit from
7880
// any fresh session on an unbound root opens the sheet again.
7981
if (currentSessionRef.current !== null) return;
8082
const p = event.payload;
@@ -194,12 +196,13 @@ export function WorkspaceBindingSheet() {
194196
New workspace detected
195197
</div>
196198
<h2 className="text-[22px] font-semibold leading-tight tracking-tight text-[rgb(var(--foreground))]">
197-
Which tools should this folder get?
199+
This folder is using your Starter set
198200
</h2>
199201
<p className="mt-2 text-sm text-[rgb(var(--muted))]">
200202
You just opened this folder in a connected app. It&apos;s already
201-
using your default Starter tools — pick a Space and feature set to
202-
give it a specific set instead, or keep the default.
203+
configured with your default Starter tools. Pick a different Space
204+
or feature set below to change what it gets, or close to keep the
205+
Starter.
203206
</p>
204207

205208
<div className="mt-5 flex items-start gap-3 rounded-xl border border-[rgb(var(--border))] bg-[rgb(var(--surface))] px-4 py-3">
@@ -282,17 +285,18 @@ export function WorkspaceBindingSheet() {
282285
{error}
283286
</div>
284287
)}
285-
{/* "Not now" auto-sizes to its label; the primary action takes
286-
the rest of the row. Equal flex-1 columns wrapped the longer
287-
"Remember for this folder" text onto two lines. */}
288+
{/* "Close" keeps the default Starter set (nothing written); the
289+
primary "Modify" applies the picked Space + feature set as an
290+
explicit mapping. Labels deliberately avoid "Not now", which read
291+
as "this folder is unmapped / has no tools" — it isn't. */}
288292
<div className="flex gap-2">
289293
<Button
290294
variant="secondary"
291295
className="px-5"
292296
onClick={handleDismiss}
293297
disabled={saving}
294298
>
295-
Not now
299+
Close
296300
</Button>
297301
<Button
298302
variant="primary"
@@ -305,7 +309,7 @@ export function WorkspaceBindingSheet() {
305309
) : (
306310
<Check className="mr-1.5 h-4 w-4" />
307311
)}
308-
Remember for this folder
312+
Modify
309313
</Button>
310314
</div>
311315
<p className="mt-3 text-center text-[11px] text-[rgb(var(--muted))]">

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ import { invoke } from '@tauri-apps/api/core';
77
* - `custom`: user-defined.
88
*/
99
/**
10-
* `starter` is the auto-seeded FS that comes with each Space. It has no
11-
* special routing role under resolver v3 — bindings and per-client grants
12-
* pick FeatureSets explicitly. The legacy `'default'` value is accepted on
13-
* read because migration 013 rewrites stored rows lazily and a stale fetch
14-
* could still surface it; new writes use `'starter'`.
10+
* `starter` is the auto-seeded FS that comes with each Space. It's the
11+
* 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'`.
1516
*/
1617
export type FeatureSetType = 'starter' | 'default' | 'custom';
1718

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
//! FeatureSet entity - permission bundles for tools/prompts/resources
22
//!
33
//! Each FeatureSet is scoped to a space and is one of two types:
4-
//! - **Starter**: auto-created with the Space as a convenient starting
5-
//! point. Has no special routing role under the resolver — bindings and
6-
//! per-client grants pick FeatureSets explicitly. Pre-resolver-v3 this
7-
//! was the "Default" type and acted as the implicit fallback; that
8-
//! behaviour is gone, and the rename reflects the type's actual job
9-
//! (a seed you can rename, edit, or delete freely).
4+
//! - **Starter**: auto-created with the Space. It's the **default fallback**
5+
//! 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.)
1011
//! - **Custom**: any other operator-defined FeatureSet.
1112
1213
use chrono::{DateTime, Utc};
@@ -16,15 +17,17 @@ use uuid::Uuid;
1617
/// The type of a FeatureSet.
1718
///
1819
/// `Starter` is auto-created once per Space; `Custom` covers everything
19-
/// else. Routing-wise the two are interchangeable — the type tag is
20-
/// purely a UI affordance ("this one came pre-seeded with the Space").
20+
/// else. The type tag carries routing weight: the Starter is the default
21+
/// fallback the resolver routes unmapped/rootless sessions to, and it's
22+
/// builtin (not deletable).
2123
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
2224
#[serde(rename_all = "kebab-case")]
2325
#[derive(Default)]
2426
pub enum FeatureSetType {
25-
/// Auto-created with the Space. Editable / deletable like any other
26-
/// FS — no special routing semantics. Was historically called
27-
/// `Default` (DB column value carried over via migration 013).
27+
/// 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).
2831
Starter,
2932
/// Any operator-defined FeatureSet.
3033
#[default]
@@ -237,9 +240,10 @@ impl FeatureSet {
237240
id: format!("fs_default_{}", space_id),
238241
name: "Starter".to_string(),
239242
description: Some(
240-
"Auto-created with this Space. Edit, rename, or delete freely \
241-
— bindings and per-client grants pick FeatureSets explicitly, \
242-
so this one has no special routing role."
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."
243247
.to_string(),
244248
),
245249
icon: Some("⭐".to_string()),

crates/mcpmux-storage/src/database.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ const MIGRATIONS: &[Migration] = &[
118118
name: "purge_orphaned_feature_set_members",
119119
sql: include_str!("migrations/017_purge_orphaned_feature_set_members.sql"),
120120
},
121+
Migration {
122+
version: 18,
123+
name: "starter_is_default_fallback_copy",
124+
sql: include_str!("migrations/018_starter_is_default_fallback_copy.sql"),
125+
},
121126
];
122127

123128
/// SQLite database wrapper.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
-- Migration 018: the Starter FS is the default fallback again (and can't be
2+
-- deleted).
3+
--
4+
-- After the "default FeatureSet for unmapped roots" change, an unmapped folder
5+
-- (plus rootless / unknown sessions) falls back to the default Space's Starter
6+
-- FS instead of being denied. That makes the Starter load-bearing: it's the
7+
-- default toolset for anything not explicitly mapped, and it is no longer
8+
-- deletable. Migrations 014/015 (and the seed paths) set a description that
9+
-- now lies — "no special routing role; delete freely" — so rewrite it.
10+
--
11+
-- Safety: only rewrite rows that STILL match the exact 014/015 seed text, so
12+
-- an operator who customized the copy keeps their change.
13+
14+
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.'
16+
WHERE is_builtin = 1
17+
AND feature_set_type IN ('starter', 'default')
18+
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/space_repository.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,12 @@ impl SpaceRepository for SqliteSpaceRepository {
109109
// Auto-seed the builtin "Starter" FeatureSet for this Space — a
110110
// ready-to-use starting point. The id prefix `fs_default_<space>`
111111
// is preserved for FK-stability across the rename (migration 013).
112-
// No special routing role under resolver v3 — bindings and per-
113-
// client grants pick FeatureSets explicitly.
112+
// The Starter is the default fallback for folders that aren't
113+
// explicitly mapped (and rootless/unknown sessions), so it's
114+
// load-bearing and builtin (can't be deleted).
114115
conn.execute(
115116
"INSERT OR IGNORE INTO feature_sets (id, name, description, icon, space_id, feature_set_type, is_builtin, created_at, updated_at)
116-
VALUES (?1, 'Starter', '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.', '⭐', ?2, 'starter', 1, ?3, ?3)",
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)",
117118
params![
118119
format!("fs_default_{}", space_id),
119120
space_id,

tests/rust/tests/database/migrations.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,59 @@ fn test_017_purges_orphaned_feature_set_members() {
9595
"only the still-resolvable feature member should survive the purge"
9696
);
9797
}
98+
99+
/// Migration 018 rewrites the auto-seeded Starter FS's now-stale "no special
100+
/// routing role; delete freely" description (the Starter is the default
101+
/// fallback again and can't be deleted), but only on rows that STILL match the
102+
/// exact 014/015 seed text — a Starter an operator re-described keeps its copy.
103+
#[test]
104+
fn test_018_rewrites_stale_starter_description_only() {
105+
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.";
107+
108+
let test_db = TestDatabase::new();
109+
let conn = test_db.db.connection();
110+
111+
// Two builtin Starter rows: one still carrying the stale 014/015 copy
112+
// (should be rewritten), one an operator customized (must be left alone).
113+
conn.execute_batch(
114+
"INSERT INTO spaces (id,name,icon,description,is_default,sort_order,created_at,updated_at)
115+
VALUES ('s-stale','Stale','x','',0,0,datetime('now'),datetime('now')),
116+
('s-cust','Custom','x','',0,0,datetime('now'),datetime('now'));
117+
INSERT INTO feature_sets
118+
(id,name,description,icon,space_id,feature_set_type,server_id,is_builtin,is_deleted,created_at,updated_at)
119+
VALUES
120+
('fs_default_s-stale','Starter',
121+
'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.',
122+
'⭐','s-stale','starter',NULL,1,0,datetime('now'),datetime('now')),
123+
('fs_default_s-cust','Starter','My own words','⭐','s-cust','starter',NULL,1,0,datetime('now'),datetime('now'));",
124+
)
125+
.expect("seed failed");
126+
127+
// Re-apply migration 018 (idempotent) to exercise the rewrite.
128+
conn.execute_batch(include_str!(
129+
"../../../../crates/mcpmux-storage/src/migrations/018_starter_is_default_fallback_copy.sql"
130+
))
131+
.expect("migration 018 failed");
132+
133+
let desc = |id: &str| -> String {
134+
conn.query_row(
135+
"SELECT description FROM feature_sets WHERE id = ?",
136+
[id],
137+
|r| r.get::<_, String>(0),
138+
)
139+
.unwrap()
140+
};
141+
142+
assert_eq!(
143+
desc("fs_default_s-stale"),
144+
FRESH,
145+
"stale copy should be rewritten"
146+
);
147+
assert_ne!(STALE, FRESH); // guard against the strings drifting equal
148+
assert_eq!(
149+
desc("fs_default_s-cust"),
150+
"My own words",
151+
"operator-customized copy must be preserved"
152+
);
153+
}

0 commit comments

Comments
 (0)