perf: add inventory cache for stateless server patterns

[SamMorrowDrums]

Summary

Add CachedInventory to build tool/resource/prompt definitions once at startup rather than per-request. This is particularly useful for the remote server pattern where a new server instance is created per request.

Problem

In stateless deployments (like the remote MCP server), a new server and inventory is created for each incoming request. This means AllTools(t), AllResources(t), and AllPrompts(t) are called repeatedly, rebuilding all ~130 tool definitions including JSON schema generation on every request.

Related: modelcontextprotocol/go-sdk#685

Solution

Add a CachedInventory that:

• Builds all tools/resources/prompts once at startup
• Uses sync.Once for thread-safe initialization
• Returns pre-populated builders for per-request use

API

// Initialize once at startup (typically in main() or server init)
github.InitInventoryCache(translator)

// Then on each request, get a builder with pre-cached tools
inv := github.CachedInventoryBuilder().
    WithReadOnly(cfg.ReadOnly).
    WithToolsets(cfg.EnabledToolsets).
    WithFeatureChecker(checker).  // Still evaluated per-request!
    Build()

Key Features

• InitInventoryCache(t) - Called once at startup with your translator
• CachedInventoryBuilder() - Returns a builder pre-populated with cached definitions
• Per-request configuration still works - read-only, toolsets, feature flags, filters
• Thread-safe via sync.Once
• Backward compatible - NewInventory(t) still works without caching

Why This Works

The elegance is preserved because:

1. Tools are still self-describing - The GetMe(t), GetTeams(t) pattern stays the same
2. No code changes needed for tool definitions - They work as before
3. Feature flags work per-request - They're evaluated in AvailableTools(ctx), not at definition time
4. Translations resolved once - Since remote server uses NullTranslationHelper anyway, this is fine

For Remote Server Integration

// In server startup/init (once)
func init() {
    github.InitInventoryCache(translations.NullTranslationHelper)
}

// Per-request handler
func handleRequest(ctx context.Context, req Request) {
    inv := github.CachedInventoryBuilder().
        WithReadOnly(cfg.ReadOnly).
        WithToolsets(cfg.Toolsets).
        WithFeatureChecker(createFeatureChecker(ctx, req.User)).
        Build()
    
    // Feature flags are evaluated here, per-request
    tools := inv.AvailableTools(ctx)
    // ...
}

Testing

• Added comprehensive unit tests in inventory_cache_test.go
• All existing tests pass
• script/lint passes

[Copilot]

Pull request overview

This PR adds an inventory caching mechanism to optimize performance in stateless server deployments where a new server instance is created per request. The cache eliminates redundant rebuilding of ~130 tool definitions and their JSON schemas on every request.

Key Changes

• Introduces CachedInventory with sync.Once for thread-safe one-time initialization of tool/resource/prompt definitions
• Provides InitInventoryCache() for explicit startup initialization and CachedInventoryBuilder() for cached builder access
• Maintains full backward compatibility with existing NewInventory() API

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
pkg/github/inventory_cache.go	Implements the caching mechanism with global singleton, sync.Once initialization, and builder factory methods
pkg/github/inventory_cache_test.go	Comprehensive test coverage including initialization, idempotency, auto-initialization, per-request filtering, and comparison with uncached behavior

[Copilot]

The ResetInventoryCache function creates a race condition. After resetting globalInventoryCache to a new instance, the old sync.Once in the previous instance is lost. If another goroutine was in the middle of calling init() on the old instance, it would complete initialization on the old instance while new callers would get the new uninitialized instance. Additionally, resetting doesn't prevent the old sync.Once from keeping the "already called" state.

While the comment correctly warns this is test-only and not thread-safe, a safer approach would be to reset the fields inside the existing CachedInventory struct rather than replacing the entire global variable. However, sync.Once cannot be reset by design.

Consider adding a mutex-based solution for tests or document more clearly that tests must ensure serial execution when calling ResetInventoryCache.

[Copilot]

Tests in this file intentionally do not use t.Parallel() because they mutate global state via ResetInventoryCache(). This is correct, but it should be documented with a comment at the top of the test file to explain why these tests must run serially, especially since most other test files in this package use t.Parallel().

Suggested change

	"github.com/stretchr/testify/require"
	// NOTE: Tests in this file intentionally do NOT use t.Parallel().
	// They mutate process-wide inventory cache state via ResetInventoryCache /
	// InitInventoryCache / CachedInventoryBuilder, so running them in parallel
	// could cause test flakiness and data races. Keep these tests serial even
	// though most other tests in this package use t.Parallel().

[Copilot]

The IsCacheInitialized check is unreliable. It checks if len(globalInventoryCache.tools) > 0, but this has two problems:

1. If AllTools(t) returns an empty slice (which shouldn't happen in practice but is theoretically possible), IsCacheInitialized would return false even after successful initialization
2. After ResetInventoryCache() is called, the check looks at the new instance which will have empty tools, making it work correctly. However, the comment at line 106 says "We can't directly check sync.Once state" which suggests the intent is to check if the Once has fired, not just if tools are populated.

A more reliable approach would be to add an explicit boolean flag that's set to true inside the once.Do callback, which would accurately reflect whether initialization has occurred regardless of whether AllTools returns an empty slice.

[Copilot]

The documentation is contradictory. Line 76 states "The cache must be initialized via InitInventoryCache before calling this function" but line 77 immediately says "If the cache is not initialized, this will initialize it with NullTranslationHelper."

This creates confusion about whether initialization is required or optional. Consider rephrasing to clarify the behavior, such as: "The cache should typically be initialized via InitInventoryCache before calling this function. If not already initialized, it will automatically initialize with NullTranslationHelper as a fallback."

[Copilot]

Comment uses inconsistent terminology. The comment says "NewInventoryBuilder" but the actual function is called "CachedInventoryBuilder". This creates confusion about which function is being referred to.

Suggested change

	// only once, regardless of how many times NewInventoryBuilder is called.
	// only once, regardless of how many times CachedInventoryBuilder is called.