fix(everything): make syncRoots idempotent on error and empty paths

[1060996408]

Description

syncRoots in src/everything/server/roots.ts claims (in JSDoc) to be idempotent and fetch roots from the client at most once per session, but the previous implementation only populated the cache on the success path. If listRoots threw or the client returned a falsy / unshaped response, no entry was written to the cache. Subsequent syncRoots calls would then re-issue roots/list to the client and re-register the notifications/roots/list_changed handler — once per tool invocation while the error persisted.

This change makes the cache-write semantics match the documented contract.

Server Details

• Server: everything (reference server)
• Changes to: server/roots.ts (+ new tests/roots.test.ts)

Motivation and Context

Two concrete issues with the old code:

1. Repeated listRoots requests on transient client errors. Every call to get-roots-list (or any caller of syncRoots) re-hit the client. With a flaky client this turns one user-initiated tool call into N silent client round-trips.
2. Repeated setNotificationHandler registration. server.server.setNotificationHandler(RootsListChangedNotificationSchema, requestRoots) is invoked on every retry. Whether this is a no-op (last-write-wins) or accumulates depends on SDK internals — either way it shouldn't be re-registered.

The JSDoc already promises "This function is idempotent. It should only request roots from the client once per session." The fix aligns the code with that promise rather than weakening the doc.

What changed

In requestRoots:

• success path: unchanged (roots.set(sessionId, response.roots) + log).
• falsy / unshaped response: now caches an empty array so the session is marked synced.
• throw path: caches an empty array (only if no entry yet) so subsequent calls don't retry; still logs to stderr.

In syncRoots:

• early-return when the client lacks the roots capability (one less indent level for the rest).
• before await requestRoots(), write a placeholder roots.set(sessionId, []). This guards against a re-entrant call landing while the initial fetch is still in flight (the second caller would otherwise see !roots.has(sessionId) and install a second notification handler).
• successful response replaces the placeholder.

Refresh continues to come from the roots/list_changed notification channel — that handler is registered exactly once per session.

How Has This Been Tested?

New test file src/everything/__tests__/roots.test.ts (4 tests):

• caches roots and does not re-fetch on subsequent calls (success path; baseline)
• does not re-fetch when listRoots throws (before fix: 3 calls, after: 1)
• does not re-fetch when listRoots returns falsy / unshaped (before fix: 2 calls, after: 1)
• skips fetch entirely when the client lacks the roots capability

Full suite for src/everything: 99 / 99 pass (95 existing + 4 new). npm run build clean.

Manual: re-ran get-roots-list against the local everything server with a working client; behavior unchanged on the success path.

Breaking Changes

None. Internal cache semantics only; the public tool surface (get-roots-list) returns the same shape, including the empty-list-with-helper-text branch when the client has no roots configured.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature
• Breaking change
• Documentation update (JSDoc updated alongside the behavioral fix)

Checklist

• I have read the MCP Protocol Documentation
• My changes follow MCP security best practices
• I have updated the server's README accordingly (no user-facing change)
• I have tested this with an LLM client
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have documented all environment variables and configuration options (none changed)

Additional context

• 1 commit, 2 files (server/roots.ts, new __tests__/roots.test.ts), +148 / −53.
• No new dependencies, no protocol-level change.