Skip to content

Commit af600c8

Browse files
committed
fix(routing): close root-fetch race + Starter editability + binding autosave
Five fixes pulled from a session-mapping debugging pass. 1. on_initialized retries list_roots(). The single-shot peer.list_roots() in the spawned init task had no retry. A transient transport blip left the session at PendingRoots forever, and roots-capable sessions saw only the meta tools. Loop with backoffs 100ms / 300ms / 800ms / 2s / 5s = 6 attempts, ~8s budget, retrying only on transport errors (Ok([]) is a valid 'no folder open' answer the client will follow up with a roots/list_changed when it has one). 2. On-demand probe in list_tools / list_prompts / list_resources. If a roots-capable session hits a list request before its init list_roots() landed (the visible 'Claude shows only 4 meta tools after opening a new workspace' bug), fire a 300ms-budget probe here, populate session_roots, then resolve routing. SessionRootsRegistry gains claim_probe(sid, throttle) so a burst of three list calls doesn't fan out three upstream peer.list_roots() — only the first in any 1s window wins. 3. Migration 015 rewrites the *other* legacy seeded copy. Migration 014 only caught the 'The fallback feature set for this space' variant set by space_repository.rs. The Default Space's row was seeded by migration 001 itself with 'Features automatically granted to all connected clients in this space' — directly the opposite of what's true under resolver v3. 015 rewrites that string with the same is_builtin + name + description guard so any operator-customized copy survives. 4. Starter FSes are editable. The two member-modification guards in commands/feature_set.rs (add_feature_set_member, set_feature_set_members) rejected feature_set_type='starter' because they only accepted 'default' and 'custom'. Result: the auto-seeded Starter FS was read-only — useless. Both guards now accept 'starter' (and keep 'default' as a legacy alias for any stale read pre-migration-013). Comment updated. 5. WorkspaceBinding autosave debounce + flush-on-close. Bumped debounce 600ms to 1500ms to coalesce multi-FS toggle bursts into one save. Stricter dedupe (compares against last-saved, not just initial — re-toggling A → B → A is a true no-op). Most importantly, pending edits now survive panel close: a separate unmount-only useEffect reads pendingPayloadRef and posts the IPC immediately if there's unsaved work, so closing the sheet right after typing no longer drops the change. Latest onSubmit / onSaveStatusChange are kept in refs so the unmount handler uses the freshest closures. cargo check + clippy (-D warnings) + pnpm typecheck all clean. Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 98bceb8 commit af600c8

6 files changed

Lines changed: 408 additions & 88 deletions

File tree

apps/desktop/src-tauri/src/commands/feature_set.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,9 +314,14 @@ pub async fn add_feature_set_member(
314314
.map_err(|e| e.to_string())?
315315
.ok_or("Feature set not found")?;
316316

317-
// Only "default" and "custom" types can have their members modified
317+
// Both Starter (auto-seeded) and Custom FeatureSets are member-driven
318+
// and editable. Reject anything else — there are no other configurable
319+
// types today, but the guard stays for forward compatibility.
320+
// `'default'` is accepted as a legacy alias because `parse('default')`
321+
// resolves to `Starter` and `as_str()` always emits `'starter'` post-
322+
// migration 013, but older in-memory data could still surface it.
318323
let fs_type = feature_set.feature_set_type.as_str();
319-
if fs_type != "default" && fs_type != "custom" {
324+
if fs_type != "starter" && fs_type != "default" && fs_type != "custom" {
320325
return Err(format!(
321326
"Cannot modify members of '{}' type feature set",
322327
fs_type
@@ -453,12 +458,13 @@ pub async fn set_feature_set_members(
453458
.map_err(|e| e.to_string())?
454459
.ok_or("Feature set not found")?;
455460

456-
// Only "default" and "custom" types can have their members modified
457-
// "all" grants everything automatically, "server-all" is also auto-computed
461+
// Both Starter (auto-seeded) and Custom FeatureSets are member-driven
462+
// and editable. `'default'` is accepted as a legacy alias for the same
463+
// reason described in `add_feature_set_member` — see comment there.
458464
let fs_type = feature_set.feature_set_type.as_str();
459-
if fs_type != "default" && fs_type != "custom" {
465+
if fs_type != "starter" && fs_type != "default" && fs_type != "custom" {
460466
return Err(format!(
461-
"Cannot modify members of '{}' type feature set. Only 'default' and 'custom' types are configurable.",
467+
"Cannot modify members of '{}' type feature set. Only Starter and Custom FeatureSets are configurable.",
462468
fs_type
463469
));
464470
}

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

Lines changed: 104 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,23 @@ function formatFsList(names: string[]): string {
455455
return names.filter((n) => n && n.length > 0).join(' + ');
456456
}
457457

458+
/**
459+
* Structural equality between two binding inputs. The autosave effect
460+
* uses this to skip writes when the user re-toggled their way back to
461+
* the last-saved state — avoids spamming `WorkspaceBindingChanged` for
462+
* a no-op edit. `feature_set_ids` order matters (it's the operator-
463+
* chosen render order, not just a set), so we compare positionally.
464+
*/
465+
function sameBindingInput(
466+
a: WorkspaceBindingInput,
467+
b: { workspace_root: string; space_id: string; feature_set_ids: string[] }
468+
): boolean {
469+
if (a.workspace_root.trim() !== b.workspace_root.trim()) return false;
470+
if (a.space_id !== b.space_id) return false;
471+
if (a.feature_set_ids.length !== b.feature_set_ids.length) return false;
472+
return a.feature_set_ids.every((id, i) => id === b.feature_set_ids[i]);
473+
}
474+
458475
function SegmentedFilter<T extends string>({
459476
value,
460477
onChange,
@@ -1638,35 +1655,75 @@ function BindingForm({
16381655
}
16391656
};
16401657

1641-
// Auto-save in edit mode — debounced, sequence-numbered to discard stale
1642-
// saves, and a no-op while the form's contents still match the initial
1643-
// values so just opening the panel doesn't fire a write.
1658+
// ---------- Autosave (edit mode) -----------------------------------------
1659+
//
1660+
// Debounced (1500 ms) so a burst of FS-toggle clicks coalesces into one
1661+
// save instead of firing N WorkspaceBindingChanged events back-to-back.
1662+
// Dedupe is against the **last successfully-saved** payload, not just
1663+
// `initial` — so re-toggling A → B → A is a no-op (back to last saved),
1664+
// and once a save lands the next idle window doesn't re-save the same
1665+
// values.
1666+
//
1667+
// Critical: the debounce timer is cleared on dependency change but the
1668+
// **pending payload survives panel close**. If the user edits then
1669+
// closes before the debounce fires, the unmount handler flushes the
1670+
// save synchronously to Tauri — the IPC goes out before React tears
1671+
// the component down, and the save completes in the background.
16441672
const saveSeqRef = useRef(0);
16451673
const savedTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
1674+
// Snapshot of the last payload we successfully wrote. `null` means
1675+
// "never saved during this panel session" — fall back to `initial` for
1676+
// dedupe in that case.
1677+
const lastSavedRef = useRef<WorkspaceBindingInput | null>(null);
1678+
// The most recent payload the user produced that has NOT yet been
1679+
// committed. Cleared on successful save. The unmount handler reads
1680+
// this to decide whether to flush.
1681+
const pendingPayloadRef = useRef<WorkspaceBindingInput | null>(null);
1682+
// Latest closures via ref so the unmount-only effect's empty-deps
1683+
// cleanup can still call the freshest handlers — closing the panel
1684+
// mid-edit must use the parent's *current* `onSubmit`, not whatever it
1685+
// captured on first mount.
1686+
const onSubmitRef = useRef(onSubmit);
1687+
const onSaveStatusChangeRef = useRef(onSaveStatusChange);
1688+
useEffect(() => {
1689+
onSubmitRef.current = onSubmit;
1690+
onSaveStatusChangeRef.current = onSaveStatusChange;
1691+
}, [onSubmit, onSaveStatusChange]);
1692+
16461693
useEffect(() => {
16471694
if (!isEdit || !initial) return;
1648-
const sameFs =
1649-
fsIds.length === initial.feature_set_ids.length &&
1650-
fsIds.every((id, i) => id === initial.feature_set_ids[i]);
1651-
const same =
1652-
root.trim() === initial.workspace_root &&
1653-
spaceId === initial.space_id &&
1654-
sameFs;
1655-
if (same) return;
16561695
if (!canSubmit) return;
1696+
1697+
const candidate: WorkspaceBindingInput = {
1698+
workspace_root: root.trim(),
1699+
space_id: spaceId,
1700+
feature_set_ids: fsIds,
1701+
};
1702+
1703+
// Dedupe baseline: last-saved if we've saved during this session,
1704+
// otherwise the initial payload from when the panel opened.
1705+
const baseline = lastSavedRef.current ?? {
1706+
workspace_root: initial.workspace_root,
1707+
space_id: initial.space_id,
1708+
feature_set_ids: initial.feature_set_ids,
1709+
};
1710+
if (sameBindingInput(candidate, baseline)) {
1711+
pendingPayloadRef.current = null;
1712+
return;
1713+
}
1714+
1715+
pendingPayloadRef.current = candidate;
16571716
const seq = ++saveSeqRef.current;
16581717
onSaveStatusChange?.({ kind: 'idle' });
16591718
const handle = setTimeout(async () => {
16601719
if (saveSeqRef.current !== seq) return;
16611720
onSaveStatusChange?.({ kind: 'saving' });
16621721
setSubmitting(true);
16631722
try {
1664-
await onSubmit({
1665-
workspace_root: root.trim(),
1666-
space_id: spaceId,
1667-
feature_set_ids: fsIds,
1668-
});
1723+
await onSubmit(candidate);
16691724
if (saveSeqRef.current !== seq) return;
1725+
lastSavedRef.current = candidate;
1726+
pendingPayloadRef.current = null;
16701727
onSaveStatusChange?.({ kind: 'saved' });
16711728
if (savedTimerRef.current) clearTimeout(savedTimerRef.current);
16721729
savedTimerRef.current = setTimeout(() => {
@@ -1680,7 +1737,7 @@ function BindingForm({
16801737
} finally {
16811738
setSubmitting(false);
16821739
}
1683-
}, 600);
1740+
}, 1500);
16841741
return () => clearTimeout(handle);
16851742
}, [
16861743
isEdit,
@@ -1694,6 +1751,36 @@ function BindingForm({
16941751
onSaveStatusChange,
16951752
]);
16961753

1754+
// Unmount-only flush. If a save was scheduled but the timer hasn't
1755+
// fired by the time the user closes the panel, fire it now so their
1756+
// edits aren't silently dropped. Empty-deps so this only runs on
1757+
// unmount, not on every dep change of the autosave effect above.
1758+
useEffect(() => {
1759+
return () => {
1760+
const pending = pendingPayloadRef.current;
1761+
if (!pending) return;
1762+
// Fire-and-forget. Tauri's `invoke` posts the IPC message to the
1763+
// Rust side immediately; the React tree can unmount in parallel
1764+
// and the save still completes. Bump the seq so any in-flight
1765+
// debounced save from before the close is discarded if it lands.
1766+
saveSeqRef.current += 1;
1767+
onSaveStatusChangeRef.current?.({ kind: 'saving' });
1768+
onSubmitRef
1769+
.current(pending)
1770+
.then(() => {
1771+
onSaveStatusChangeRef.current?.({ kind: 'saved' });
1772+
})
1773+
.catch((e) => {
1774+
// Parent's toast bridge is gone with the panel — fall back to
1775+
// the console so the failure isn't silent in dev.
1776+
console.warn(
1777+
'[workspace-binding] flush-on-close save failed:',
1778+
e instanceof Error ? e.message : String(e)
1779+
);
1780+
});
1781+
};
1782+
}, []);
1783+
16971784
const submitLabel =
16981785
mode === 'create-from-live' ? 'Save binding' : 'Create binding';
16991786

0 commit comments

Comments
 (0)