diff --git a/.github/badges/coverage.svg b/.github/badges/coverage.svg index bfc9c9db..c742bb27 100644 --- a/.github/badges/coverage.svg +++ b/.github/badges/coverage.svg @@ -1 +1 @@ -coverage: 49.8%coverage49.8% \ No newline at end of file +coverage: 52.8%coverage52.8% \ No newline at end of file diff --git a/internal/app/plugin_stdio_overlay.go b/internal/app/plugin_stdio_overlay.go new file mode 100644 index 00000000..5d0f7057 --- /dev/null +++ b/internal/app/plugin_stdio_overlay.go @@ -0,0 +1,206 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + "encoding/json" + "log/slog" + "os" + "path/filepath" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cache" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/compat" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/plugin" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/transport" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/config" + "github.com/spf13/cobra" +) + +// resolveStdioOverlay resolves the CLIOverlay for a stdio plugin server +// from its manifest. It supports two forms: +// +// 1. inline JSON object in manifest.MCPServers[key].CLI +// 2. a relative file path (JSON string) pointing to an external overlay +// file anchored at the plugin root (e.g. "overlay.json") +// +// When no CLI metadata is present, a minimal overlay keyed by the server +// name is returned so callers can still build an identity descriptor. +func resolveStdioOverlay(p *plugin.Plugin, sc plugin.StdioServerClient) market.CLIOverlay { + serverID := sc.Key + overlay := market.CLIOverlay{ + ID: serverID, + Command: serverID, + } + srv, ok := p.Manifest.MCPServers[sc.Key] + if !ok || len(srv.CLI) == 0 { + return overlay + } + + cliData := srv.CLI + // A JSON string is interpreted as a relative path to an external + // overlay file (e.g. "overlay.json") anchored at the plugin root. + if len(cliData) > 0 && cliData[0] == '"' { + var cliPath string + if err := json.Unmarshal(cliData, &cliPath); err == nil && cliPath != "" { + absPath := filepath.Join(p.Root, cliPath) + if fileData, readErr := os.ReadFile(absPath); readErr == nil { + cliData = fileData + } else { + slog.Warn("plugin: failed to read CLI overlay file", + "plugin", p.Manifest.Name, "path", absPath, "error", readErr) + } + } + } + if err := json.Unmarshal(cliData, &overlay); err != nil { + slog.Warn("plugin: failed to parse CLI overlay for stdio server", + "plugin", p.Manifest.Name, "server", sc.Key, "error", err) + } + if overlay.ID == "" { + overlay.ID = serverID + } + if overlay.Command == "" { + overlay.Command = serverID + } + return overlay +} + +// toolsToDetails converts discovered ToolDescriptors to the DetailTool map +// shape expected by compat.BuildDynamicCommands (keyed by overlay ID). +// Returns nil if tools is empty. +func toolsToDetails(tools []transport.ToolDescriptor, overlayID string) map[string][]market.DetailTool { + if len(tools) == 0 { + return nil + } + detailTools := make([]market.DetailTool, 0, len(tools)) + for _, tool := range tools { + schemaJSON := "" + if tool.InputSchema != nil { + if data, marshalErr := json.Marshal(tool.InputSchema); marshalErr == nil { + schemaJSON = string(data) + } + } + detailTools = append(detailTools, market.DetailTool{ + ToolName: tool.Name, + ToolTitle: tool.Title, + ToolDesc: tool.Description, + IsSensitive: tool.Sensitive, + ToolRequest: schemaJSON, + }) + } + return map[string][]market.DetailTool{overlayID: detailTools} +} + +// registerStdioServerFromOverlay builds cobra commands for a stdio plugin +// server using only its manifest + overlay.json — no subprocess required. +// +// Returns (cmds, descriptor, true) when the overlay carries toolOverrides, +// otherwise (nil, zero, false) so the caller can fall back to discovery-first +// registration (legacy path). +// +// When a warm tools cache exists for this server, its DetailTools are passed +// to BuildDynamicCommands so flag types are enriched from the last successful +// discovery. Fresh installs (or evicted caches) get overlay-declared flags +// only; the next startup after a successful refresh picks up the full schema. +func registerStdioServerFromOverlay( + p *plugin.Plugin, + sc plugin.StdioServerClient, + runner executor.Runner, + store *cache.Store, +) ([]*cobra.Command, market.ServerDescriptor, bool) { + overlay := resolveStdioOverlay(p, sc) + if len(overlay.ToolOverrides) == 0 { + return nil, market.ServerDescriptor{}, false + } + + descriptor := market.ServerDescriptor{ + Key: sc.Key, + DisplayName: p.Manifest.Name + "/" + sc.Key, + Description: p.Manifest.Description, + Endpoint: StdioEndpoint(p.Manifest.Name, sc.Key), + Source: "plugin", + CLI: overlay, + HasCLIMeta: true, + } + + AppendDynamicServer(descriptor) + RegisterStdioClient(p.Manifest.Name+"/"+sc.Key, sc.Client) + + // Warm-cache enrichment: if a prior successful discovery wrote a + // non-empty tool list, use its schema to enrich flag types. + var detailsByID map[string][]market.DetailTool + if store != nil { + cacheKey := pluginCacheKey(p.Manifest.Name, sc.Key) + if snapshot, _, err := store.LoadTools(config.DefaultPartition, cacheKey); err == nil && len(snapshot.Tools) > 0 { + detailsByID = toolsToDetails(snapshot.Tools, overlay.ID) + } + } + + cmds := compat.BuildDynamicCommands( + []market.ServerDescriptor{descriptor}, runner, detailsByID) + + slog.Debug("plugin: stdio server registered from overlay", + "plugin", p.Manifest.Name, "server", sc.Key, + "toolOverrides", len(overlay.ToolOverrides), + "commands", len(cmds), + "enriched", detailsByID != nil) + + return cmds, descriptor, true +} + +// refreshStdioToolsCache performs Initialize + ListTools on a stdio plugin +// subprocess and persists the result so the next startup can enrich +// overlay-registered commands with typed flags. It never constructs cobra +// commands; command registration has already happened synchronously from +// the overlay before this function runs. +// +// On failure (subprocess not ready, RPC timeout, empty tool list) it skips +// SaveTools entirely so a transient error cannot poison the warm cache +// with a null-tools snapshot. +func refreshStdioToolsCache( + p *plugin.Plugin, + sc plugin.StdioServerClient, + store *cache.Store, + timeouts pluginColdTimeouts, +) { + if store == nil { + return + } + tools := discoverStdioTools(p, sc, timeouts) + if len(tools) == 0 { + slog.Debug("plugin: stdio cache refresh skipped (no tools)", + "plugin", p.Manifest.Name, "server", sc.Key) + return + } + cacheKey := pluginCacheKey(p.Manifest.Name, sc.Key) + if err := store.SaveTools(config.DefaultPartition, cacheKey, cache.ToolsSnapshot{ + ServerKey: cacheKey, + Tools: tools, + }); err != nil { + slog.Warn("plugin: failed to persist stdio tools cache", + "plugin", p.Manifest.Name, "server", sc.Key, "error", err) + return + } + slog.Debug("plugin: stdio tools cache refreshed", + "plugin", p.Manifest.Name, "server", sc.Key, "tools", len(tools)) +} + +// hasOverlayToolOverrides reports whether a stdio plugin server carries +// enough CLI metadata to be registered via the overlay-first path. Used by +// loadPlugins to split entries into overlay-first vs. legacy discovery-first +// buckets without doing the overlay parse twice. +func hasOverlayToolOverrides(p *plugin.Plugin, sc plugin.StdioServerClient) bool { + return len(resolveStdioOverlay(p, sc).ToolOverrides) > 0 +} diff --git a/internal/app/plugin_stdio_overlay_test.go b/internal/app/plugin_stdio_overlay_test.go new file mode 100644 index 00000000..d449898b --- /dev/null +++ b/internal/app/plugin_stdio_overlay_test.go @@ -0,0 +1,398 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + "encoding/json" + "testing" + "time" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cache" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/plugin" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/transport" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/config" + "github.com/spf13/cobra" +) + +// withCleanStdioRegistry snapshots and restores the package-level stdio +// client registry so tests that call RegisterStdioClient don't leak state +// across cases. +func withCleanStdioRegistry(t *testing.T) { + t.Helper() + stdioMu.Lock() + prev := stdioClients + stdioClients = make(map[string]*transport.StdioClient) + stdioMu.Unlock() + t.Cleanup(func() { + stdioMu.Lock() + stdioClients = prev + stdioMu.Unlock() + }) +} + +// newOverlayFixture constructs a plugin + stdio entry carrying an inline +// CLIOverlay with the given tool-override map. The stdio client is created +// but never started, since the overlay-first path does not require the +// subprocess to be running for command registration. +func newOverlayFixture(t *testing.T, pluginName, serverKey string, overlay market.CLIOverlay) (*plugin.Plugin, plugin.StdioServerClient) { + t.Helper() + raw, err := json.Marshal(overlay) + if err != nil { + t.Fatalf("marshal overlay: %v", err) + } + p := &plugin.Plugin{ + Manifest: plugin.Manifest{ + Name: pluginName, + Version: "1.0.0", + Description: pluginName + " plugin", + MCPServers: map[string]*plugin.MCPServer{ + serverKey: { + Type: "stdio", + Command: "/usr/bin/true", // never executed by overlay-first path + CLI: raw, + }, + }, + }, + Root: t.TempDir(), + } + sc := plugin.StdioServerClient{ + Key: serverKey, + Client: transport.NewStdioClient("/usr/bin/true", nil, nil), + } + return p, sc +} + +// TestRegisterStdioServerFromOverlay_NoDiscoveryStillBuildsCommands verifies +// the core promise of the overlay-first path: when overlay.json ships +// ToolOverrides, commands appear immediately — no subprocess probe. +func TestRegisterStdioServerFromOverlay_NoDiscoveryStillBuildsCommands(t *testing.T) { + withCleanDynamicRegistry(t) + withCleanStdioRegistry(t) + + overlay := market.CLIOverlay{ + ID: "conference-local", + Command: "conference-local", + Groups: map[string]market.CLIGroupDef{ + "meeting": {Description: "会议控制"}, + "member": {Description: "成员管理"}, + }, + ToolOverrides: map[string]market.CLIToolOverride{ + "create_meeting": {CLIName: "create", Group: "meeting", Description: "Create a meeting"}, + "end_meeting": {CLIName: "end", Group: "meeting", Description: "End a meeting"}, + "mute_member": {CLIName: "mute", Group: "member", Description: "Mute a member"}, + }, + } + p, sc := newOverlayFixture(t, "conference-local", "conference-local", overlay) + + store := cache.NewStore(t.TempDir()) + cmds, desc, ok := registerStdioServerFromOverlay(p, sc, executor.EchoRunner{}, store) + if !ok { + t.Fatal("registerStdioServerFromOverlay returned ok=false, want true") + } + if len(cmds) == 0 { + t.Fatal("registerStdioServerFromOverlay returned 0 commands, want >=1") + } + + var root *struct{ name, path string } + _ = root + found := false + for _, c := range cmds { + if c.Name() == "conference-local" { + found = true + // Groups must be attached as sub-commands. + groups := map[string]bool{} + for _, sub := range c.Commands() { + groups[sub.Name()] = true + } + if !groups["meeting"] { + t.Errorf("missing 'meeting' group sub-command, children = %v", groups) + } + if !groups["member"] { + t.Errorf("missing 'member' group sub-command, children = %v", groups) + } + } + } + if !found { + names := []string{} + for _, c := range cmds { + names = append(names, c.Name()) + } + t.Fatalf("missing top-level 'conference-local' command, got %v", names) + } + + // AppendDynamicServer registration: product ID should land in + // DirectRuntimeProductIDs so hideNonDirectRuntimeCommands keeps it + // visible even under a restrictive VisibleProducts hook. + if !DirectRuntimeProductIDs()["conference-local"] { + t.Error("DirectRuntimeProductIDs missing 'conference-local'") + } + + // RegisterStdioClient side-effect: the runtime must be able to look up + // the StdioClient when the endpoint is invoked later. + if _, ok := LookupStdioClient("conference-local/conference-local"); !ok { + t.Error("LookupStdioClient missing conference-local/conference-local") + } + + if desc.Endpoint != StdioEndpoint("conference-local", "conference-local") { + t.Errorf("descriptor.Endpoint = %q, want %q", desc.Endpoint, StdioEndpoint("conference-local", "conference-local")) + } +} + +// TestRegisterStdioServerFromOverlay_WarmCacheEnrichesFlags pre-populates the +// tools cache with a schema-bearing DetailTool and asserts the resulting +// leaf command picks up the typed flag derived from InputSchema. +func TestRegisterStdioServerFromOverlay_WarmCacheEnrichesFlags(t *testing.T) { + withCleanDynamicRegistry(t) + withCleanStdioRegistry(t) + + overlay := market.CLIOverlay{ + ID: "cache-plugin", + Command: "cache-plugin", + ToolOverrides: map[string]market.CLIToolOverride{ + "echo": {CLIName: "echo", Description: "Echo input"}, + }, + } + p, sc := newOverlayFixture(t, "cache-plugin", "cache-plugin", overlay) + + store := cache.NewStore(t.TempDir()) + cacheKey := pluginCacheKey(p.Manifest.Name, sc.Key) + if err := store.SaveTools(config.DefaultPartition, cacheKey, cache.ToolsSnapshot{ + SavedAt: time.Now().UTC(), + ServerKey: cacheKey, + Tools: []transport.ToolDescriptor{ + { + Name: "echo", + Description: "Echo the input", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "message": map[string]any{"type": "string"}, + }, + "required": []any{"message"}, + }, + }, + }, + }); err != nil { + t.Fatalf("SaveTools: %v", err) + } + + cmds, _, ok := registerStdioServerFromOverlay(p, sc, executor.EchoRunner{}, store) + if !ok || len(cmds) == 0 { + t.Fatalf("overlay registration failed: ok=%v cmds=%d", ok, len(cmds)) + } + + var echoLeaf *leafMatch + for _, top := range cmds { + if top.Name() != "cache-plugin" { + continue + } + for _, sub := range top.Commands() { + if sub.Name() == "echo" { + echoLeaf = &leafMatch{name: sub.Name(), hasFlag: sub.Flags().Lookup("message") != nil} + } + } + } + if echoLeaf == nil { + t.Fatal("missing 'echo' leaf command under 'cache-plugin'") + } + if !echoLeaf.hasFlag { + t.Error("warm-cache enrichment did not wire --message flag from InputSchema") + } +} + +type leafMatch struct { + name string + hasFlag bool +} + +// TestRegisterStdioServerFromOverlay_OverlayWithoutOverridesReturnsFalse +// asserts the fallback contract: when overlay.json is missing toolOverrides, +// the overlay-first path declines so the caller can route the entry through +// the legacy discovery-first registerStdioServer. +func TestRegisterStdioServerFromOverlay_OverlayWithoutOverridesReturnsFalse(t *testing.T) { + withCleanDynamicRegistry(t) + withCleanStdioRegistry(t) + + // Overlay with no ToolOverrides (simulates a plugin that relies entirely + // on runtime discovery for its tool list). + overlay := market.CLIOverlay{ + ID: "legacy-plugin", + Command: "legacy-plugin", + } + p, sc := newOverlayFixture(t, "legacy-plugin", "legacy-plugin", overlay) + + store := cache.NewStore(t.TempDir()) + cmds, _, ok := registerStdioServerFromOverlay(p, sc, executor.EchoRunner{}, store) + if ok { + t.Errorf("registerStdioServerFromOverlay ok=true for empty toolOverrides; want false") + } + if cmds != nil { + t.Errorf("cmds = %v, want nil", cmds) + } + if DirectRuntimeProductIDs()["legacy-plugin"] { + t.Error("legacy-plugin must NOT be appended to dynamic registry in fallback case") + } + if _, found := LookupStdioClient("legacy-plugin/legacy-plugin"); found { + t.Error("stdio client must NOT be registered in fallback case") + } +} + +// TestRefreshStdioToolsCache_FailurePreservesCache guards against the +// "negative cache poisoning" bug: if discovery fails (subprocess not ready, +// timeout, empty tool list), the existing warm cache must remain intact so +// the next startup still enriches flags from the last good snapshot. +func TestRefreshStdioToolsCache_FailurePreservesCache(t *testing.T) { + withCleanDynamicRegistry(t) + withCleanStdioRegistry(t) + + p, sc := newOverlayFixture(t, "refresh-plugin", "refresh-plugin", market.CLIOverlay{ + ID: "refresh-plugin", + Command: "refresh-plugin", + }) + + store := cache.NewStore(t.TempDir()) + cacheKey := pluginCacheKey(p.Manifest.Name, sc.Key) + goodSnapshot := cache.ToolsSnapshot{ + SavedAt: time.Now().UTC(), + ServerKey: cacheKey, + Tools: []transport.ToolDescriptor{ + { + Name: "ping", + Description: "Health check", + InputSchema: map[string]any{"type": "object"}, + }, + }, + } + if err := store.SaveTools(config.DefaultPartition, cacheKey, goodSnapshot); err != nil { + t.Fatalf("seed SaveTools: %v", err) + } + + // /usr/bin/true exits immediately, so Initialize + ListTools will fail + // (no MCP handshake). discoverStdioTools returns nil → refresh must be + // a no-op and must NOT overwrite the good cache with a null snapshot. + refreshStdioToolsCache(p, sc, store, pluginColdTimeouts{stdio: 200 * time.Millisecond}) + + got, _, err := store.LoadTools(config.DefaultPartition, cacheKey) + if err != nil { + t.Fatalf("LoadTools after failed refresh: %v", err) + } + if len(got.Tools) != 1 || got.Tools[0].Name != "ping" { + t.Errorf("warm cache was overwritten by failed refresh: %+v", got.Tools) + } +} + +// TestLoadPlugins_OverlayFirstVisibleBeforeDiscovery is an integration-style +// test for the loadPlugins split decision: stdio plugins whose overlay ships +// ToolOverrides must have their commands visible on the root immediately, +// WITHOUT waiting on any discovery handshake. It drives the same sequence +// loadPlugins uses (registerStdioServerFromOverlay → root.AddCommand → +// hideNonDirectRuntimeCommands) and asserts the plugin command survives the +// visibility filter even when no discovery has run. +func TestLoadPlugins_OverlayFirstVisibleBeforeDiscovery(t *testing.T) { + withCleanDynamicRegistry(t) + withCleanStdioRegistry(t) + + // Simulate a wukong-like edition that declares a static VisibleProducts + // whitelist NOT containing our plugin. This is the exact scenario where + // the original bug surfaced. + overrideVisibleProducts(t, []string{"calendar", "doc"}) + + overlay := market.CLIOverlay{ + ID: "conference-local", + Command: "conference-local", + ToolOverrides: map[string]market.CLIToolOverride{ + "create_meeting": {CLIName: "create", Description: "Create a meeting"}, + }, + } + p, sc := newOverlayFixture(t, "conference-local", "conference-local", overlay) + + // No discovery runs — no cache seeded. This mirrors a cold-start where + // the subprocess is unavailable (or just slow) yet the user expects + // `dws --help` to still list the plugin. + store := cache.NewStore(t.TempDir()) + cmds, _, ok := registerStdioServerFromOverlay(p, sc, executor.EchoRunner{}, store) + if !ok { + t.Fatal("registerStdioServerFromOverlay returned ok=false") + } + + root := &cobra.Command{Use: "dws"} + // Also add a sibling command that is NOT a registered product so we can + // prove the visibility filter still hides non-product commands. + bogus := &cobra.Command{Use: "bogus-not-a-product"} + root.AddCommand(bogus) + for _, c := range cmds { + root.AddCommand(c) + } + + hideNonDirectRuntimeCommands(root) + + var pluginCmd *cobra.Command + for _, c := range root.Commands() { + if c.Name() == "conference-local" { + pluginCmd = c + } + } + if pluginCmd == nil { + t.Fatal("conference-local missing from root after overlay-first registration") + } + if pluginCmd.Hidden { + t.Error("conference-local must stay visible (Hidden=false) after hideNonDirectRuntimeCommands") + } + if !bogus.Hidden { + t.Error("bogus-not-a-product must be hidden by the visibility filter") + } + + services := visibleMCPRootCommands(root) + if !containsCommand(services, "conference-local") { + t.Errorf("visibleMCPRootCommands missing conference-local: %v", commandNames(services)) + } +} + +// TestHasOverlayToolOverrides exercises the split-decision helper used by +// loadPlugins to route stdio entries to overlay-first vs. legacy buckets. +func TestHasOverlayToolOverrides(t *testing.T) { + cases := []struct { + name string + overlay market.CLIOverlay + want bool + }{ + { + name: "empty overlay", + overlay: market.CLIOverlay{ID: "x", Command: "x"}, + want: false, + }, + { + name: "overlay with overrides", + overlay: market.CLIOverlay{ + ID: "x", + Command: "x", + ToolOverrides: map[string]market.CLIToolOverride{ + "foo": {CLIName: "foo"}, + }, + }, + want: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p, sc := newOverlayFixture(t, "x", "x", tc.overlay) + got := hasOverlayToolOverrides(p, sc) + if got != tc.want { + t.Errorf("hasOverlayToolOverrides = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/internal/app/root.go b/internal/app/root.go index d61a06e2..ec567f74 100644 --- a/internal/app/root.go +++ b/internal/app/root.go @@ -655,16 +655,7 @@ func newMCPCommand(ctx context.Context, loader cli.CatalogLoader, runner executo // Public utility commands (auth, cache, completion, version) are always kept // visible; explicitly hidden commands stay hidden. func hideNonDirectRuntimeCommands(root *cobra.Command) { - var allowedProducts map[string]bool - if fn := edition.Get().VisibleProducts; fn != nil { - products := fn() - allowedProducts = make(map[string]bool, len(products)) - for _, p := range products { - allowedProducts[p] = true - } - } else { - allowedProducts = DirectRuntimeProductIDs() - } + allowedProducts := resolveVisibleProducts() staticCommands := map[string]bool{ "auth": true, "cache": true, @@ -1162,11 +1153,31 @@ func loadPlugins(engine *pipeline.Engine, runner executor.Runner) []*cobra.Comma sharedStore := cacheStoreFromEnv() coldTimeouts := resolvePluginColdTimeouts() - // Fan out HTTP and stdio discovery in parallel. Each goroutine resolves - // its cache hit locally (no network) or runs a bounded cold-path probe. - // Wall-clock cost ≈ max(individual plugin latencies), not the sum. + // Phase A: stdio overlay-first registration (synchronous, no I/O). + // Plugins whose overlay.json declares ToolOverrides register their full + // command tree up-front from manifest metadata alone — no subprocess + // handshake required. This fixes the "discovery fails → no commands + // ever appear" lock-out and keeps `dws --help` reliable even when the + // underlying MCP server is temporarily unavailable. + var legacyStdioEntries []stdioEntry + for _, e := range stdioEntries { + cmds, _, ok := registerStdioServerFromOverlay(e.plugin, e.sc, runner, sharedStore) + if !ok { + legacyStdioEntries = append(legacyStdioEntries, e) + continue + } + pluginCmds = append(pluginCmds, cmds...) + } + + // Phase B: fan out discovery in parallel. + // - HTTP plugins: same behaviour as before (discovery-first). + // - stdio overlay-first plugins: async cache refresh only; their + // commands are already registered. Failures are non-fatal and do + // NOT poison the warm-cache with a null-tools snapshot. + // - stdio legacy plugins (overlay without toolOverrides): preserve + // the old discovery-first path for backwards compatibility. httpResults := make([][]*cobra.Command, len(httpServers)) - stdioResults := make([][]*cobra.Command, len(stdioEntries)) + legacyStdioResults := make([][]*cobra.Command, len(legacyStdioEntries)) var wg sync.WaitGroup for i, ps := range httpServers { wg.Add(1) @@ -1175,18 +1186,30 @@ func loadPlugins(engine *pipeline.Engine, runner executor.Runner) []*cobra.Comma httpResults[idx] = registerHTTPServer(ps.plugin, ps.srv, tc, runner, sharedStore, coldTimeouts) }(i, ps) } - for i, e := range stdioEntries { + // overlay-first stdio: async refresh (no command building here). + for _, e := range stdioEntries { + if !hasOverlayToolOverrides(e.plugin, e.sc) { + continue + } + wg.Add(1) + go func(e stdioEntry) { + defer wg.Done() + refreshStdioToolsCache(e.plugin, e.sc, sharedStore, coldTimeouts) + }(e) + } + // legacy stdio: discovery-first (commands depend on tool list). + for i, e := range legacyStdioEntries { wg.Add(1) go func(idx int, e stdioEntry) { defer wg.Done() - stdioResults[idx] = registerStdioServer(e.plugin, e.sc, runner, sharedStore, coldTimeouts) + legacyStdioResults[idx] = registerStdioServer(e.plugin, e.sc, runner, sharedStore, coldTimeouts) }(i, e) } wg.Wait() for _, cmds := range httpResults { pluginCmds = append(pluginCmds, cmds...) } - for _, cmds := range stdioResults { + for _, cmds := range legacyStdioResults { pluginCmds = append(pluginCmds, cmds...) } @@ -1472,6 +1495,11 @@ func discoverStdioTools(p *plugin.Plugin, sc plugin.StdioServerClient, timeouts // buildStdioCommands constructs Cobra commands from a tool list and // registers the runtime dispatch state (StdioClient + dynamic server). // Returns nil for an empty tool list. +// +// This is the legacy discovery-first path, used only for stdio plugins whose +// overlay.json does NOT carry toolOverrides. Plugins that ship toolOverrides +// register commands up-front via registerStdioServerFromOverlay, bypassing +// this function entirely (see plugin_stdio_overlay.go). func buildStdioCommands(p *plugin.Plugin, sc plugin.StdioServerClient, tools []transport.ToolDescriptor, runner executor.Runner) []*cobra.Command { if len(tools) == 0 { slog.Debug("plugin: stdio server has no tools", @@ -1479,44 +1507,14 @@ func buildStdioCommands(p *plugin.Plugin, sc plugin.StdioServerClient, tools []t return nil } - // Build CLIOverlay: use manifest CLI metadata if present, else auto-generate. - serverID := sc.Key - overlay := market.CLIOverlay{ - ID: serverID, - Command: serverID, - } - if srv, ok := p.Manifest.MCPServers[sc.Key]; ok && len(srv.CLI) > 0 { - cliData := srv.CLI - // If cli is a JSON string, treat it as a relative file path to an overlay file. - if len(cliData) > 0 && cliData[0] == '"' { - var cliPath string - if err := json.Unmarshal(cliData, &cliPath); err == nil && cliPath != "" { - absPath := filepath.Join(p.Root, cliPath) - if fileData, readErr := os.ReadFile(absPath); readErr == nil { - cliData = fileData - } else { - slog.Warn("plugin: failed to read CLI overlay file", - "plugin", p.Manifest.Name, "path", absPath, "error", readErr) - } - } - } - if err := json.Unmarshal(cliData, &overlay); err != nil { - slog.Warn("plugin: failed to parse CLI overlay for stdio server", - "plugin", p.Manifest.Name, "server", sc.Key, "error", err) - } - if overlay.ID == "" { - overlay.ID = serverID - } - if overlay.Command == "" { - overlay.Command = serverID - } - } + overlay := resolveStdioOverlay(p, sc) - // Auto-generate ToolOverrides from discovered tools when not provided. + // Auto-generate ToolOverrides from discovered tools when not provided + // by the manifest/overlay (legacy discovery-first path). if len(overlay.ToolOverrides) == 0 { overlay.ToolOverrides = make(map[string]market.CLIToolOverride) if len(overlay.Prefixes) == 0 { - overlay.Prefixes = []string{serverID} + overlay.Prefixes = []string{overlay.ID} } for _, tool := range tools { overlay.ToolOverrides[tool.Name] = market.CLIToolOverride{ @@ -1525,43 +1523,20 @@ func buildStdioCommands(p *plugin.Plugin, sc plugin.StdioServerClient, tools []t } } - // Construct virtual endpoint and server descriptor. - endpoint := StdioEndpoint(p.Manifest.Name, sc.Key) - descriptor := market.ServerDescriptor{ Key: sc.Key, DisplayName: p.Manifest.Name + "/" + sc.Key, Description: p.Manifest.Description, - Endpoint: endpoint, + Endpoint: StdioEndpoint(p.Manifest.Name, sc.Key), Source: "plugin", CLI: overlay, HasCLIMeta: true, } AppendDynamicServer(descriptor) - // Register with pluginName/serverKey format for cleanup by plugin name - RegisterStdioClient(p.Manifest.Name+"/"+serverID, sc.Client) - - // Convert tool descriptors to DetailTool entries for flag generation. - detailsByID := make(map[string][]market.DetailTool) - var detailTools []market.DetailTool - for _, tool := range tools { - schemaJSON := "" - if tool.InputSchema != nil { - if data, marshalErr := json.Marshal(tool.InputSchema); marshalErr == nil { - schemaJSON = string(data) - } - } - detailTools = append(detailTools, market.DetailTool{ - ToolName: tool.Name, - ToolTitle: tool.Title, - ToolDesc: tool.Description, - IsSensitive: tool.Sensitive, - ToolRequest: schemaJSON, - }) - } - detailsByID[serverID] = detailTools + RegisterStdioClient(p.Manifest.Name+"/"+sc.Key, sc.Client) + detailsByID := toolsToDetails(tools, overlay.ID) cmds := compat.BuildDynamicCommands( []market.ServerDescriptor{descriptor}, runner, detailsByID) diff --git a/internal/app/root_help.go b/internal/app/root_help.go index 2380dea7..711b8176 100644 --- a/internal/app/root_help.go +++ b/internal/app/root_help.go @@ -84,21 +84,30 @@ func renderRootHelp(root *cobra.Command) { _, _ = fmt.Fprintln(w, `Use "dws --help" for more information about a discovered MCP service or "dws --help" for utility commands.`) } +// resolveVisibleProducts returns the set of top-level product IDs that should +// be treated as visible. It unions the edition's VisibleProducts hook (when +// set) with DirectRuntimeProductIDs(), so dynamically-registered products — +// including plugins loaded via AppendDynamicServer — are never silently hidden +// by a static VisibleProducts list. +func resolveVisibleProducts() map[string]bool { + allowed := map[string]bool{} + if fn := edition.Get().VisibleProducts; fn != nil { + for _, p := range fn() { + allowed[p] = true + } + } + for id := range DirectRuntimeProductIDs() { + allowed[id] = true + } + return allowed +} + func visibleMCPRootCommands(root *cobra.Command) []*cobra.Command { if root == nil { return nil } - var allowed map[string]bool - if fn := edition.Get().VisibleProducts; fn != nil { - products := fn() - allowed = make(map[string]bool, len(products)) - for _, p := range products { - allowed[p] = true - } - } else { - allowed = DirectRuntimeProductIDs() - } + allowed := resolveVisibleProducts() if len(allowed) == 0 { return nil } @@ -121,13 +130,7 @@ func visibleUtilityRootCommands(root *cobra.Command) []*cobra.Command { return nil } - productCommands := DirectRuntimeProductIDs() - if fn := edition.Get().VisibleProducts; fn != nil { - productCommands = make(map[string]bool, len(fn())) - for _, product := range fn() { - productCommands[product] = true - } - } + productCommands := resolveVisibleProducts() commands := make([]*cobra.Command, 0) for _, cmd := range root.Commands() { diff --git a/internal/app/visibility_test.go b/internal/app/visibility_test.go new file mode 100644 index 00000000..bd0866d0 --- /dev/null +++ b/internal/app/visibility_test.go @@ -0,0 +1,125 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" + "github.com/spf13/cobra" +) + +// overrideVisibleProducts temporarily installs an edition hook exposing the +// given static product list and restores the previous hooks on cleanup. +func overrideVisibleProducts(t *testing.T, products []string) { + t.Helper() + prev := edition.Get() + edition.Override(&edition.Hooks{ + VisibleProducts: func() []string { return products }, + }) + t.Cleanup(func() { edition.Override(prev) }) +} + +// registerPluginProduct simulates a plugin's `AppendDynamicServer` call so +// the product ID ends up in DirectRuntimeProductIDs() without triggering +// network discovery. +func registerPluginProduct(t *testing.T, id, endpoint string) { + t.Helper() + AppendDynamicServer(market.ServerDescriptor{ + Endpoint: endpoint, + CLI: market.CLIOverlay{ + ID: id, + Command: id, + }, + }) +} + +// TestHideNonDirectRuntimeCommands_PluginVisibleDespiteStaticVisibleProducts +// is a regression for the dws-wukong plugin-visibility bug: when an edition +// installs a static VisibleProducts hook (Wukong returns 40 hardcoded product +// IDs) and a plugin registers a new product via AppendDynamicServer +// (e.g. `conference-local`), the plugin command must stay visible because the +// dynamic registry takes precedence over the hook's static whitelist. +func TestHideNonDirectRuntimeCommands_PluginVisibleDespiteStaticVisibleProducts(t *testing.T) { + withCleanDynamicRegistry(t) + overrideVisibleProducts(t, []string{"calendar"}) + registerPluginProduct(t, "conference-local", "stdio://plugin/conference-local") + + root := &cobra.Command{Use: "dws"} + calendarCmd := &cobra.Command{Use: "calendar"} + pluginCmd := &cobra.Command{Use: "conference-local"} + bogusCmd := &cobra.Command{Use: "bogus-not-a-product"} + root.AddCommand(calendarCmd, pluginCmd, bogusCmd) + + hideNonDirectRuntimeCommands(root) + + if calendarCmd.Hidden { + t.Errorf("calendar (static VisibleProducts) must stay visible, got Hidden=true") + } + if pluginCmd.Hidden { + t.Errorf("conference-local (plugin-registered) must stay visible, got Hidden=true") + } + if !bogusCmd.Hidden { + t.Errorf("bogus-not-a-product must be hidden, got Hidden=false") + } +} + +// TestVisibleMCPRootCommands_IncludesPluginProducts asserts that the help +// renderer surfaces plugin products in the "Discovered MCP Services" section +// and does not misclassify them as utility commands. +func TestVisibleMCPRootCommands_IncludesPluginProducts(t *testing.T) { + withCleanDynamicRegistry(t) + overrideVisibleProducts(t, []string{"calendar"}) + registerPluginProduct(t, "conference-local", "stdio://plugin/conference-local") + + root := &cobra.Command{Use: "dws"} + calendarCmd := &cobra.Command{Use: "calendar"} + pluginCmd := &cobra.Command{Use: "conference-local"} + authCmd := &cobra.Command{Use: "auth"} + root.AddCommand(calendarCmd, pluginCmd, authCmd) + + services := visibleMCPRootCommands(root) + if !containsCommand(services, "conference-local") { + t.Errorf("visibleMCPRootCommands missing plugin command: %v", commandNames(services)) + } + if !containsCommand(services, "calendar") { + t.Errorf("visibleMCPRootCommands missing static product: %v", commandNames(services)) + } + + utilities := visibleUtilityRootCommands(root) + if containsCommand(utilities, "conference-local") { + t.Errorf("visibleUtilityRootCommands must not include plugin command, got %v", commandNames(utilities)) + } + if !containsCommand(utilities, "auth") { + t.Errorf("visibleUtilityRootCommands must include genuine utility command, got %v", commandNames(utilities)) + } +} + +func containsCommand(cmds []*cobra.Command, name string) bool { + for _, c := range cmds { + if c.Name() == name { + return true + } + } + return false +} + +func commandNames(cmds []*cobra.Command) []string { + names := make([]string, 0, len(cmds)) + for _, c := range cmds { + names = append(names, c.Name()) + } + return names +}