Move resolveEnabledToolsets to Inventory

[tommaso-moro]

Summary

Moves the resolveEnabledToolsets logic from internal/ghmcp/server.go into the Inventory Builder (pkg/inventory/builder.go), continuing the architectural direction followed in #1869 where we give the server a configured inventory instead of creating one as part of the server creation.

Tested locally.

Why

This aligns with the architecture where the Inventory holds all pre-computed configuration (the MCP server should receive a fully-configured inventory). This also makes the logic reusable for library consumers (e.g., remote server) without duplicating the toolset resolution logic.

Changes

• Added dynamicMode field and WithDynamicMode(bool) method to Builder
• Added applyDynamicModePreprocessing() that handles:
  • Removing "all" and "default" keywords in dynamic mode
  • Starting with empty toolsets when dynamic mode is enabled with no explicit toolsets
  • Using empty toolsets (not defaults) when specific tools are requested without toolsets
• Updated NewMCPServer to pass raw config values and call WithDynamicMode(cfg.DynamicToolsets)
• Removed resolveEnabledToolsets() from server.go

MCP impact

• No tool or API changes
• Tool schema or behavior changed
• New tool added

Prompts tested (tool changes only)

Security / limits

• No security or limits impact
• Auth / permissions considered
• Data exposure, filtering, or token/size limits considered

Tool renaming

• I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
  • I have added the new tool aliases in deprecated_tool_aliases.go
• I am not renaming tools as part of this PR

Note: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.

Lint & tests

• Linted locally with ./script/lint
• Tested locally with ./script/test

Docs

• Not needed
• Updated (README / docs / examples)

[Copilot]

Pull request overview

Moves enabled-toolset resolution out of internal/ghmcp/server.go and into the Inventory builder so the server receives a precomputed, fully-configured Inventory (continuing the architecture introduced in #1869).

Changes:

• Added dynamicMode / WithDynamicMode(bool) and dynamic-mode preprocessing in pkg/inventory/builder.go.
• Updated NewMCPServer to pass raw config values into the inventory builder and enable dynamic mode via WithDynamicMode(cfg.DynamicToolsets).
• Replaced the removed resolveEnabledToolsets() coverage with new inventory-level tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
pkg/inventory/builder.go	Adds dynamic toolset-mode preprocessing and integrates it into Build().
pkg/inventory/registry_test.go	Adds tests covering dynamic mode and “tools without toolsets” behavior.
internal/ghmcp/server.go	Stops resolving toolsets in the server; delegates to inventory builder configuration.
internal/ghmcp/server_test.go	Removes server-level tests for resolveEnabledToolsets() after refactor.

Comments suppressed due to low confidence (1)

pkg/inventory/builder.go:186

• applyDynamicModePreprocessing checks len(b.additionalTools) > 0 before WithTools() input is cleaned. If WithTools([]string{"", " "}) is used, this path forces toolsetIDs to [] (disabling defaults) even though cleanTools() would later drop all additional tools. Consider cleaning additionalTools (or at least checking for any non-empty trimmed entries) before deciding to override toolset behavior.

	if len(b.additionalTools) > 0 {
		// When specific tools are requested but no toolsets, don't use default toolsets
		// --tools=X alone registers only X
		b.toolsetIDs = []string{}
		b.toolsetIDsIsNil = false
		return

[Copilot]

In dynamic mode, applyDynamicModePreprocessing removes the special keywords using exact string comparison ("all" / "default") before any trimming. If callers pass values like " all " or " default " (which processToolsets() later trims), these keywords won’t be stripped and dynamic mode can unexpectedly behave like "all"/"default" were requested. Consider normalizing (e.g., strings.TrimSpace + compare) when stripping, or handling the dynamic-mode keyword suppression inside processToolsets() where trimming already occurs.

This issue also appears in the following locations of the same file:

• line 181

Suggested change

	// applyDynamicModePreprocessing modifies toolsetIDs based on dynamicMode and additionalTools.
	func (b *Builder) applyDynamicModePreprocessing() {
	if b.dynamicMode && b.toolsetIDs != nil {
	// In dynamic mode, remove "all" and "default" since users enable toolsets on demand
	b.toolsetIDs = removeFromSlice(b.toolsetIDs, "all")
	b.toolsetIDs = removeFromSlice(b.toolsetIDs, "default")
	// removeFromSliceTrimmed removes all occurrences of value from slice,
	// comparing after trimming whitespace from both the slice elements and value.
	func removeFromSliceTrimmed(slice []string, value string) []string {
	trimmedValue := strings.TrimSpace(value)
	result := make([]string, 0, len(slice))
	for _, s := range slice {
	if strings.TrimSpace(s) != trimmedValue {
	result = append(result, s)
	}
	}
	return result
	}
	// applyDynamicModePreprocessing modifies toolsetIDs based on dynamicMode and additionalTools.
	func (b *Builder) applyDynamicModePreprocessing() {
	if b.dynamicMode && b.toolsetIDs != nil {
	// In dynamic mode, remove "all" and "default" since users enable toolsets on demand.
	// Use trimmed comparison so values like " all " are treated the same as "all".
	b.toolsetIDs = removeFromSliceTrimmed(b.toolsetIDs, "all")
	b.toolsetIDs = removeFromSliceTrimmed(b.toolsetIDs, "default")