From 7692048cf4eeed98217ca64475558340b97ab2b3 Mon Sep 17 00:00:00 2001 From: xianfeng wang <522096432@qq.com> Date: Thu, 30 Apr 2026 09:44:25 +0800 Subject: [PATCH] fix(discovery): surface edition supplement/fallback servers in runtime catalog loader (#197) * chore: update coverage badge [skip ci] * chore: update coverage badge [skip ci] * chore: update coverage badge [skip ci] * fix conference discovery --------- Co-authored-by: github-actions[bot] --- .github/badges/coverage.svg | 2 +- internal/app/legacy.go | 66 +----- .../app/legacy_wukong_partition_e2e_test.go | 64 ++++++ internal/app/runner.go | 11 + .../app/runner_supplement_fallback_test.go | 192 ++++++++++++++++++ internal/cli/loader.go | 125 +++++++++++- internal/cli/loader_partition_test.go | 164 +++++++++++++++ internal/cli/loader_supplement_test.go | 151 ++++++++++++++ internal/editionmerge/merge.go | 87 ++++++++ internal/editionmerge/merge_test.go | 123 +++++++++++ pkg/config/constants.go | 17 ++ pkg/config/constants_test.go | 22 ++ 12 files changed, 966 insertions(+), 58 deletions(-) create mode 100644 internal/app/legacy_wukong_partition_e2e_test.go create mode 100644 internal/app/runner_supplement_fallback_test.go create mode 100644 internal/cli/loader_partition_test.go create mode 100644 internal/cli/loader_supplement_test.go create mode 100644 internal/editionmerge/merge.go create mode 100644 internal/editionmerge/merge_test.go diff --git a/.github/badges/coverage.svg b/.github/badges/coverage.svg index 728f8e31..c742bb27 100644 --- a/.github/badges/coverage.svg +++ b/.github/badges/coverage.svg @@ -1 +1 @@ -coverage: 52.8%coverage52.8% +coverage: 52.8%coverage52.8% \ No newline at end of file diff --git a/internal/app/legacy.go b/internal/app/legacy.go index d6912e73..2907f480 100644 --- a/internal/app/legacy.go +++ b/internal/app/legacy.go @@ -29,6 +29,7 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cli" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cobracmd" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/compat" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/editionmerge" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" @@ -123,13 +124,10 @@ func injectStaticServers(servers []edition.ServerInfo) { // Tests may override discoveryBaseURLOverride to redirect to a local server; // in that case the registry cache is always bypassed. // editionPartition returns the cache partition for the active edition. -// Each edition gets its own partition to prevent cross-edition data leakage. +// Thin wrapper around config.EditionPartition; kept so the many existing +// call sites in internal/app don't need to thread edition.Get() everywhere. func editionPartition() string { - name := edition.Get().Name - if name == "" || name == "open" { - return config.DefaultPartition - } - return name + "/default" + return config.EditionPartition(edition.Get().Name) } // discoveryTraceEnabled reports whether the user asked for discovery-path diagnostics. @@ -265,8 +263,8 @@ func loadDynamicCommands(ctx context.Context, runner executor.Runner) []*cobra.C if fn := edition.Get().FallbackServers; fn != nil { if fb := fn(); len(fb) > 0 { slog.Debug("loadDynamicCommands: using FallbackServers", "count", len(fb)) - descriptors := fallbackToDescriptors(fb) - descriptors = mergeSupplementServers(descriptors) + descriptors := editionmerge.FallbackToDescriptors(fb) + descriptors = editionmerge.MergeSupplement(descriptors) SetDynamicServers(descriptors) return nil } @@ -275,7 +273,7 @@ func loadDynamicCommands(ctx context.Context, runner executor.Runner) []*cobra.C } // Merge edition-specific supplement servers (not in Market). - servers = mergeSupplementServers(servers) + servers = editionmerge.MergeSupplement(servers) // Inject dynamic server data for endpoint resolution SetDynamicServers(servers) @@ -545,49 +543,7 @@ func mergeTopLevelCommands(commands []*cobra.Command) []*cobra.Command { return out } -// mergeSupplementServers appends edition-specific servers (not in Market) -// into the discovery result. Existing IDs from Market/cache take precedence. -func mergeSupplementServers(servers []market.ServerDescriptor) []market.ServerDescriptor { - fn := edition.Get().SupplementServers - if fn == nil { - return servers - } - existing := make(map[string]bool, len(servers)) - for _, s := range servers { - existing[s.CLI.ID] = true - existing[s.Key] = true - } - for _, sup := range fn() { - if !existing[sup.ID] { - servers = append(servers, market.ServerDescriptor{ - Key: sup.ID, - DisplayName: sup.Name, - Endpoint: sup.Endpoint, - CLI: market.CLIOverlay{ - ID: sup.ID, - Command: sup.ID, - Prefixes: sup.Prefixes, - }, - }) - } - } - return servers -} - -// fallbackToDescriptors converts edition.ServerInfo into market.ServerDescriptor. -func fallbackToDescriptors(servers []edition.ServerInfo) []market.ServerDescriptor { - descriptors := make([]market.ServerDescriptor, 0, len(servers)) - for _, s := range servers { - descriptors = append(descriptors, market.ServerDescriptor{ - Key: s.ID, - DisplayName: s.Name, - Endpoint: s.Endpoint, - CLI: market.CLIOverlay{ - ID: s.ID, - Command: s.ID, - Prefixes: s.Prefixes, - }, - }) - } - return descriptors -} +// mergeSupplementServers / fallbackToDescriptors have moved to +// internal/editionmerge so that both internal/cli and internal/app can +// apply the edition's SupplementServers / FallbackServers hooks against +// the same discovery pipeline (command tree + runtime catalog). diff --git a/internal/app/legacy_wukong_partition_e2e_test.go b/internal/app/legacy_wukong_partition_e2e_test.go new file mode 100644 index 00000000..d2ba7289 --- /dev/null +++ b/internal/app/legacy_wukong_partition_e2e_test.go @@ -0,0 +1,64 @@ +// 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/pkg/config" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +// TestEditionPartition_SingleSourceOfTruth is the regression test that +// specifically targets the original bug: internal/app.loadDynamicCommands +// was computing its partition one way (editionPartition() → +// "wukong/default") while internal/cli.EnvironmentLoader was hardcoding +// config.DefaultPartition ("default/default"). This meant runtime endpoint +// resolution and command-tree generation read different cache files, and +// under gray-release the two partitions carried disjoint product lists — +// the historical root cause of `dws conference meeting create` failing +// while `dws todo task list` succeeded on the same host. +// +// Keeping both sides funneled through config.EditionPartition is the +// central invariant the fix enforces. If this test ever regresses, the +// two-partition split almost certainly came back. +func TestEditionPartition_SingleSourceOfTruth(t *testing.T) { + t.Cleanup(func() { edition.Override(&edition.Hooks{}) }) + + cases := []struct { + name string + edition string + want string + }{ + {"open edition falls through to default/default", "", config.DefaultPartition}, + {"explicit open edition remains default", "open", config.DefaultPartition}, + {"wukong overlay uses wukong/default", "wukong", "wukong/default"}, + {"custom edition is namespaced", "internal-lab", "internal-lab/default"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + edition.Override(&edition.Hooks{Name: tc.edition}) + legacy := editionPartition() + shared := config.EditionPartition(edition.Get().Name) + + if legacy != shared { + t.Fatalf("editionPartition()=%q, config.EditionPartition()=%q — partition split regressed for edition %q", legacy, shared, tc.edition) + } + if legacy != tc.want { + t.Fatalf("editionPartition()=%q, want %q for edition %q", legacy, tc.want, tc.edition) + } + }) + } +} diff --git a/internal/app/runner.go b/internal/app/runner.go index 8b522353..819626d7 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -198,6 +198,17 @@ func (r *runtimeRunner) Run(ctx context.Context, invocation executor.Invocation) return r.handleCatalogMiss(ctx, invocation, "product missing from discovery catalog and no supplement/env override") } if _, ok := product.FindTool(invocation.Tool); !ok { + // Catalog knows the product but not the tool — this happens when the + // catalog entry came from SupplementServers (endpoint-only, no tool + // list). Trust directRuntimeEndpoint to re-resolve a working endpoint + // for the tool. If that also misses, fall through to handleCatalogMiss + // so stderr still carries the explicit not-resolved signal. + if endpoint, ok := directRuntimeEndpoint(invocation.CanonicalProduct, invocation.Tool); ok { + if r.globalFlags != nil && r.globalFlags.DryRun { + invocation.DryRun = true + } + return r.executeInvocation(ctx, endpoint, invocation) + } return r.handleCatalogMiss(ctx, invocation, fmt.Sprintf("tool %q not declared by product %q in discovery catalog", invocation.Tool, invocation.CanonicalProduct)) } if r.globalFlags != nil && r.globalFlags.DryRun { diff --git a/internal/app/runner_supplement_fallback_test.go b/internal/app/runner_supplement_fallback_test.go new file mode 100644 index 00000000..0c94dc43 --- /dev/null +++ b/internal/app/runner_supplement_fallback_test.go @@ -0,0 +1,192 @@ +// 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 ( + "context" + "strings" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/ir" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/transport" +) + +// supplementOnlyCatalogLoader mimics the post-fix EnvironmentLoader: the +// catalog has the product entry (materialised from SupplementServers) but +// no tool list — the overlay owns the tool tree locally. +type supplementOnlyCatalogLoader struct{} + +func (supplementOnlyCatalogLoader) Load(_ context.Context) (ir.Catalog, error) { + return ir.Catalog{ + Products: []ir.CanonicalProduct{ + { + ID: "conference", + ServerKey: "conference", + Endpoint: "stdio://conference-catalog", + Tools: nil, + }, + }, + }, nil +} + +func resetDynamicServers(t *testing.T) { + t.Helper() + orig := snapshotDynamicServers() + t.Cleanup(func() { restoreDynamicServers(orig) }) +} + +type dynamicServerSnapshot struct { + endpoints map[string]string + products map[string]bool + aliases map[string]string + toolEndpoints map[string]string +} + +func snapshotDynamicServers() dynamicServerSnapshot { + dynamicMu.RLock() + defer dynamicMu.RUnlock() + return dynamicServerSnapshot{ + endpoints: cloneStringMap(dynamicEndpoints), + products: cloneBoolMap(dynamicProducts), + aliases: cloneStringMap(dynamicAliases), + toolEndpoints: cloneStringMap(dynamicToolEndpoints), + } +} + +func restoreDynamicServers(s dynamicServerSnapshot) { + dynamicMu.Lock() + defer dynamicMu.Unlock() + dynamicEndpoints = s.endpoints + dynamicProducts = s.products + dynamicAliases = s.aliases + dynamicToolEndpoints = s.toolEndpoints +} + +func cloneStringMap(in map[string]string) map[string]string { + if in == nil { + return nil + } + out := make(map[string]string, len(in)) + for k, v := range in { + out[k] = v + } + return out +} + +func cloneBoolMap(in map[string]bool) map[string]bool { + if in == nil { + return nil + } + out := make(map[string]bool, len(in)) + for k, v := range in { + out[k] = v + } + return out +} + +// TestRuntimeRunner_ToolMiss_FallsBackToDirectRuntime pins the runner's +// bridge between the catalog path (where a product entry can come from +// SupplementServers with no tool list) and the direct-runtime path (which +// carries the authoritative per-tool endpoint map). When the catalog knows +// the product but not the tool, the runner should not fail-fast with +// endpoint_not_resolved — it should consult dynamicEndpoints one more time +// and proceed if an endpoint is registered. +// +// This is the narrow recovery path that keeps hardcoded overlay commands +// working under a gray-released envelope: the supplement-materialised +// catalog entry has endpoint+no tools, and SetDynamicServers holds the +// operational endpoint indexed by product / command. +func TestRuntimeRunner_ToolMiss_FallsBackToDirectRuntime(t *testing.T) { + resetDynamicServers(t) + SetDynamicServers([]market.ServerDescriptor{ + { + Key: "conference", + DisplayName: "会议", + Endpoint: "stdio://conference-fake", + CLI: market.CLIOverlay{ + ID: "conference", + Command: "conference", + }, + Source: "edition_supplement", + }, + }) + + runner := &runtimeRunner{ + loader: supplementOnlyCatalogLoader{}, + transport: transport.NewClient(nil), + fallback: executor.EchoRunner{}, + } + + // Kind = api_invocation forces the code to skip the Run() opening + // direct-runtime attempt and go through the catalog path instead, so + // the tool-miss recovery branch we're testing actually runs. + inv := executor.Invocation{ + Kind: "api_invocation", + CanonicalProduct: "conference", + Tool: "create_meeting_reservation", + CanonicalPath: "conference.create_meeting_reservation", + DryRun: true, + Params: map[string]any{}, + } + + result, err := runner.Run(context.Background(), inv) + if err != nil { + t.Fatalf("runner.Run returned error, want tool-miss fallback success: %v", err) + } + if result.Response == nil { + t.Fatalf("expected non-nil Response on dry-run") + } + if got, _ := result.Response["dry_run"].(bool); !got { + t.Fatalf("expected dry_run=true in Response, got %v", result.Response) + } + if got, _ := result.Response["transport"].(string); got != "stdio" { + t.Fatalf("expected transport=stdio in Response (proof we hit stdio://conference-fake), got %v", result.Response) + } +} + +// TestRuntimeRunner_ToolMiss_NoDynamicEntry_StillFailsClosed is the inverse +// guard: when both the catalog tool list and dynamicEndpoints have no +// record for the requested tool, the runner must still surface +// endpoint_not_resolved instead of silently producing empty output. +func TestRuntimeRunner_ToolMiss_NoDynamicEntry_StillFailsClosed(t *testing.T) { + resetDynamicServers(t) + SetDynamicServers([]market.ServerDescriptor{}) // intentionally empty + + runner := &runtimeRunner{ + loader: supplementOnlyCatalogLoader{}, + transport: transport.NewClient(nil), + fallback: executor.EchoRunner{}, + } + + inv := executor.Invocation{ + Kind: "api_invocation", + CanonicalProduct: "conference", + Tool: "nonexistent_tool", + CanonicalPath: "conference.nonexistent_tool", + Params: map[string]any{}, + } + + _, err := runner.Run(context.Background(), inv) + if err == nil { + t.Fatalf("expected endpoint_not_resolved error, got nil") + } + if !strings.Contains(err.Error(), "endpoint not resolved") { + t.Fatalf("expected endpoint_not_resolved error, got %v", err) + } + if !strings.Contains(err.Error(), "nonexistent_tool") { + t.Fatalf("error should name the missing tool; got %v", err) + } +} diff --git a/internal/cli/loader.go b/internal/cli/loader.go index e3ff5c81..828dd816 100644 --- a/internal/cli/loader.go +++ b/internal/cli/loader.go @@ -24,6 +24,7 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cache" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/discovery" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/editionmerge" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/ir" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/transport" @@ -196,14 +197,23 @@ func (l EnvironmentLoader) Load(ctx context.Context) (ir.Catalog, error) { return FixtureLoader{Path: fixturePath}.Load(ctx) } + // Priority: explicit test override > edition-specific discovery URL > + // open-source default. For Wukong this pulls the runtime catalog fetch + // onto the same Portal endpoint that loadDynamicCommands already uses, + // eliminating the historical split where the command tree came from + // Wukong Portal while runtime endpoint resolution silently read the + // open-source Market cache (see fix-wukong-endpoint-partition plan). baseURL := DefaultMarketBaseURL + if editionURL := strings.TrimSpace(edition.Get().DiscoveryURL); editionURL != "" { + baseURL = editionURL + } if l.CatalogBaseURLOverride != "" { baseURL = l.CatalogBaseURLOverride } cacheDir, _ := l.lookup(CacheDirEnv) store := cache.NewStore(cacheDir) - partition := config.DefaultPartition + partition := config.EditionPartition(edition.Get().Name) // Cache-first: if a cached catalog is available, use it immediately. // Startup command construction should not block on synchronous discovery @@ -223,6 +233,15 @@ func (l EnvironmentLoader) Load(ctx context.Context) (ir.Catalog, error) { } if !hasAuth { + // Cache / discovery both unreachable without credentials — fall back + // to the edition's SupplementServers / FallbackServers hook so that + // hardcoded overlay commands can still resolve an endpoint via the + // returned catalog. Without this an unauthenticated cold start + // produces DegradedUnauthenticated and every hardcoded command + // fails even when the edition carries its own static endpoint map. + if fb := fallbackRuntimeServers(); len(fb) > 0 { + return ir.BuildCatalog(fb), nil + } return ir.Catalog{}, newCatalogDegraded(DegradedUnauthenticated, 0) } @@ -247,6 +266,9 @@ func (l EnvironmentLoader) Load(ctx context.Context) (ir.Catalog, error) { if cached.Available && len(cached.Catalog.Products) > 0 { return cached.Catalog, nil } + if fb := fallbackRuntimeServers(); len(fb) > 0 { + return ir.BuildCatalog(fb), nil + } return ir.Catalog{}, newCatalogDegraded(DegradedMarketUnreachable, 0) } @@ -287,6 +309,9 @@ func (l EnvironmentLoader) Load(ctx context.Context) (ir.Catalog, error) { if cached.Available && len(cached.Catalog.Products) > 0 { return cached.Catalog, nil } + if fb := fallbackRuntimeServers(); len(fb) > 0 { + return ir.BuildCatalog(fb), nil + } return ir.Catalog{}, newCatalogDegraded(DegradedRuntimeAllFailed, len(servers)) } @@ -305,6 +330,7 @@ func (l EnvironmentLoader) Load(ctx context.Context) (ir.Catalog, error) { runtimeServers = append(runtimeServers, runtimeServer) } } + runtimeServers = appendSupplementRuntimeServers(runtimeServers) return ir.BuildCatalog(runtimeServers), nil } @@ -313,15 +339,28 @@ func (l EnvironmentLoader) Load(ctx context.Context) (ir.Catalog, error) { // window, the returned state asks the caller to try live discovery before // trusting the cache as current truth. func (l EnvironmentLoader) loadFromCache(store *cache.Store) cachedCatalogState { - partition := config.DefaultPartition + partition := config.EditionPartition(edition.Get().Name) regSnap, freshness, err := store.LoadRegistry(partition) if err != nil || len(regSnap.Servers) == 0 { + // No cached registry. Still honour the edition's SupplementServers + // hook so that hardcoded overlay commands whose products are not + // part of the Portal envelope (Wukong gray-release in particular) + // can resolve an endpoint via the catalog path as well. + if supplement := supplementRuntimeServers(nil); len(supplement) > 0 { + return cachedCatalogState{ + Catalog: ir.BuildCatalog(supplement), + Registry: regSnap, + Available: true, + NeedsRevalidate: true, + } + } return cachedCatalogState{} } now := store.Now().UTC() needsRevalidate := freshness == cache.FreshnessStale || cache.ShouldRevalidate(now, regSnap.SavedAt) runtimeServers := make([]discovery.RuntimeServer, 0, len(regSnap.Servers)) + existing := make(map[string]bool, len(regSnap.Servers)) for _, server := range regSnap.Servers { toolsSnap, toolsFreshness, toolsErr := store.LoadTools(partition, server.Key) if toolsErr != nil || toolsFreshness != cache.FreshnessFresh { @@ -335,10 +374,17 @@ func (l EnvironmentLoader) loadFromCache(store *cache.Store) cachedCatalogState Source: "fresh_cache", Degraded: false, }) + if id := server.CLI.ID; id != "" { + existing[id] = true + } + if server.Key != "" { + existing[server.Key] = true + } } if len(runtimeServers) != len(regSnap.Servers) { needsRevalidate = true } + runtimeServers = append(runtimeServers, supplementRuntimeServers(existing)...) return cachedCatalogState{ Catalog: ir.BuildCatalog(runtimeServers), Registry: regSnap, @@ -347,6 +393,81 @@ func (l EnvironmentLoader) loadFromCache(store *cache.Store) cachedCatalogState } } +// supplementRuntimeServers materialises the edition.SupplementServers hook +// as discovery.RuntimeServer values, skipping IDs that already appear in +// the discovery result. The returned servers carry no tools — they exist +// only so catalog.FindProduct can resolve an endpoint; tool validation +// for these products is expected to fall through to directRuntimeEndpoint. +func supplementRuntimeServers(existing map[string]bool) []discovery.RuntimeServer { + fn := edition.Get().SupplementServers + if fn == nil { + return nil + } + sup := fn() + if len(sup) == 0 { + return nil + } + out := make([]discovery.RuntimeServer, 0, len(sup)) + for _, s := range sup { + if s.ID == "" { + continue + } + if existing != nil && existing[s.ID] { + continue + } + out = append(out, discovery.RuntimeServer{ + Server: editionmerge.ToDescriptor(s, "edition_supplement"), + Source: "edition_supplement", + Degraded: false, + }) + } + return out +} + +// fallbackRuntimeServers materialises the edition.FallbackServers hook, +// additionally folding in SupplementServers entries the hook omits. +// Used when every other discovery avenue failed. +func fallbackRuntimeServers() []discovery.RuntimeServer { + fn := edition.Get().FallbackServers + if fn == nil { + return supplementRuntimeServers(nil) + } + fb := fn() + if len(fb) == 0 { + return supplementRuntimeServers(nil) + } + existing := make(map[string]bool, len(fb)) + out := make([]discovery.RuntimeServer, 0, len(fb)) + for _, s := range fb { + if s.ID == "" { + continue + } + existing[s.ID] = true + out = append(out, discovery.RuntimeServer{ + Server: editionmerge.ToDescriptor(s, "edition_fallback"), + Source: "edition_fallback", + Degraded: false, + }) + } + out = append(out, supplementRuntimeServers(existing)...) + return out +} + +// appendSupplementRuntimeServers merges supplement entries into a live +// discovery result, deduplicating against existing IDs. +func appendSupplementRuntimeServers(servers []discovery.RuntimeServer) []discovery.RuntimeServer { + existing := make(map[string]bool, len(servers)) + for _, s := range servers { + if id := s.Server.CLI.ID; id != "" { + existing[id] = true + } + if s.Server.Key != "" { + existing[s.Server.Key] = true + } + } + return append(servers, supplementRuntimeServers(existing)...) +} + func (l EnvironmentLoader) lookup(key string) (string, bool) { if l.LookupEnv == nil { return "", false diff --git a/internal/cli/loader_partition_test.go b/internal/cli/loader_partition_test.go new file mode 100644 index 00000000..ce654d64 --- /dev/null +++ b/internal/cli/loader_partition_test.go @@ -0,0 +1,164 @@ +// 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 cli + +import ( + "testing" + "time" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cache" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/ir" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +// setEdition overrides the active edition hooks for the duration of the test. +func setEdition(t *testing.T, h *edition.Hooks) { + t.Helper() + prev := edition.Get() + edition.Override(h) + t.Cleanup(func() { edition.Override(prev) }) +} + +func seedRegistryCache(t *testing.T, store *cache.Store, partition string, savedAt time.Time, servers []market.ServerDescriptor) { + t.Helper() + if err := store.SaveRegistry(partition, cache.RegistrySnapshot{ + SavedAt: savedAt, + Servers: servers, + }); err != nil { + t.Fatalf("SaveRegistry(%q) error = %v", partition, err) + } + for _, server := range servers { + if err := store.SaveTools(partition, server.Key, cache.ToolsSnapshot{ + SavedAt: savedAt, + ServerKey: server.Key, + }); err != nil { + t.Fatalf("SaveTools(%q) error = %v", server.Key, err) + } + } +} + +// TestLoadFromCache_UsesEditionPartition verifies that loadFromCache reads +// from the edition-specific partition (wukong/default) instead of the +// historical hardcoded default/default. Before the fix, an entry written to +// wukong/default was invisible to the runtime catalog loader — which is +// exactly what caused `dws conference meeting create` to report +// endpoint_not_resolved while todo succeeded (the open-source Market cache +// happened to carry todo). +func TestLoadFromCache_UsesEditionPartition(t *testing.T) { + setEdition(t, &edition.Hooks{Name: "wukong"}) + + root := t.TempDir() + now := time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC) + store := cache.NewStore(root) + store.Now = func() time.Time { return now } + + seedRegistryCache(t, store, "wukong/default", now, []market.ServerDescriptor{ + { + Key: "conference", + DisplayName: "会议", + Endpoint: "https://example.invalid/conference", + CLI: market.CLIOverlay{ + ID: "conference", + Command: "conference", + }, + }, + }) + + loader := EnvironmentLoader{} + state := loader.loadFromCache(store) + + if !state.Available { + t.Fatalf("expected cached state available; got %+v", state) + } + if _, ok := state.Catalog.FindProduct("conference"); !ok { + t.Fatalf("conference not in catalog; products=%v", productIDs(state.Catalog.Products)) + } +} + +// TestLoadFromCache_IgnoresDefaultPartitionForOverlay asserts the cross-partition +// leak is gone: writing servers under default/default while the edition is +// Wukong must NOT surface in the runtime catalog. Previously this path was +// the accidental fallback that let `dws todo` work on a gray-released host. +func TestLoadFromCache_IgnoresDefaultPartitionForOverlay(t *testing.T) { + setEdition(t, &edition.Hooks{Name: "wukong"}) + + root := t.TempDir() + now := time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC) + store := cache.NewStore(root) + store.Now = func() time.Time { return now } + + seedRegistryCache(t, store, "default/default", now, []market.ServerDescriptor{ + { + Key: "todo", + DisplayName: "待办", + Endpoint: "https://example.invalid/todo", + CLI: market.CLIOverlay{ + ID: "todo", + Command: "todo", + }, + }, + }) + + loader := EnvironmentLoader{} + state := loader.loadFromCache(store) + + if state.Available { + if _, ok := state.Catalog.FindProduct("todo"); ok { + t.Fatalf("todo leaked from default/default into wukong catalog (partition isolation regressed)") + } + } +} + +// TestLoadFromCache_OpenEdition_UsesDefaultPartition keeps the open-source +// core behaviour intact: with edition.Name == "" (zero value), loadFromCache +// must still read default/default. +func TestLoadFromCache_OpenEdition_UsesDefaultPartition(t *testing.T) { + setEdition(t, &edition.Hooks{}) + + root := t.TempDir() + now := time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC) + store := cache.NewStore(root) + store.Now = func() time.Time { return now } + + seedRegistryCache(t, store, "default/default", now, []market.ServerDescriptor{ + { + Key: "doc", + DisplayName: "文档", + Endpoint: "https://example.invalid/doc", + CLI: market.CLIOverlay{ + ID: "doc", + Command: "doc", + }, + }, + }) + + loader := EnvironmentLoader{} + state := loader.loadFromCache(store) + + if !state.Available { + t.Fatalf("expected cached state available for open edition; got %+v", state) + } + if _, ok := state.Catalog.FindProduct("doc"); !ok { + t.Fatalf("doc not in catalog; products=%v", productIDs(state.Catalog.Products)) + } +} + +func productIDs(products []ir.CanonicalProduct) []string { + ids := make([]string, 0, len(products)) + for _, p := range products { + ids = append(ids, p.ID) + } + return ids +} diff --git a/internal/cli/loader_supplement_test.go b/internal/cli/loader_supplement_test.go new file mode 100644 index 00000000..d180ab0b --- /dev/null +++ b/internal/cli/loader_supplement_test.go @@ -0,0 +1,151 @@ +// 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 cli + +import ( + "testing" + "time" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cache" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +// TestLoadFromCache_SupplementFillsGaps simulates the Wukong gray-release +// scenario: the Portal envelope only carries `live`, but the edition's +// SupplementServers hook ships the hardcoded endpoints for `conference` and +// `todo`. The resulting catalog must expose all three so runtime endpoint +// resolution does not depend on the historical default-partition accident. +func TestLoadFromCache_SupplementFillsGaps(t *testing.T) { + setEdition(t, &edition.Hooks{ + Name: "wukong", + SupplementServers: func() []edition.ServerInfo { + return []edition.ServerInfo{ + {ID: "conference", Name: "会议", Endpoint: "https://example.invalid/conference"}, + {ID: "todo", Name: "待办", Endpoint: "https://example.invalid/todo"}, + // Duplicate of the discovery entry — MUST be overridden by + // the discovery entry (discovery wins on ID collision). + {ID: "live", Name: "直播(supplement)", Endpoint: "https://example.invalid/overridden"}, + } + }, + }) + + root := t.TempDir() + now := time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC) + store := cache.NewStore(root) + store.Now = func() time.Time { return now } + + liveEndpoint := "https://example.invalid/live" + seedRegistryCache(t, store, "wukong/default", now, []market.ServerDescriptor{ + { + Key: "live", + DisplayName: "直播", + Endpoint: liveEndpoint, + CLI: market.CLIOverlay{ + ID: "live", + Command: "live", + }, + }, + }) + + loader := EnvironmentLoader{} + state := loader.loadFromCache(store) + if !state.Available { + t.Fatalf("expected cached state available; got %+v", state) + } + + wantIDs := map[string]string{ + "conference": "https://example.invalid/conference", + "todo": "https://example.invalid/todo", + "live": liveEndpoint, // discovery wins, NOT the supplement's overridden URL + } + for id, wantEndpoint := range wantIDs { + product, ok := state.Catalog.FindProduct(id) + if !ok { + t.Errorf("catalog missing product %q; have %v", id, productIDs(state.Catalog.Products)) + continue + } + if product.Endpoint != wantEndpoint { + t.Errorf("product %q endpoint = %q, want %q", id, product.Endpoint, wantEndpoint) + } + } +} + +// TestLoadFromCache_EmptyRegistry_StillExposesSupplement covers the cold-start +// gray-release case: no cached registry at all, but the edition still knows +// about a set of hardcoded products. Those should be exposed via the catalog +// so `dws foo bar` does not fail with endpoint_not_resolved on first run. +func TestLoadFromCache_EmptyRegistry_StillExposesSupplement(t *testing.T) { + setEdition(t, &edition.Hooks{ + Name: "wukong", + SupplementServers: func() []edition.ServerInfo { + return []edition.ServerInfo{ + {ID: "conference", Name: "会议", Endpoint: "https://example.invalid/conference"}, + } + }, + }) + + root := t.TempDir() + now := time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC) + store := cache.NewStore(root) + store.Now = func() time.Time { return now } + + loader := EnvironmentLoader{} + state := loader.loadFromCache(store) + + if !state.Available { + t.Fatalf("expected cached state available via supplement; got %+v", state) + } + if _, ok := state.Catalog.FindProduct("conference"); !ok { + t.Fatalf("supplement did not surface conference into catalog; products=%v", productIDs(state.Catalog.Products)) + } + if !state.NeedsRevalidate { + t.Errorf("NeedsRevalidate should be true when only supplement is available") + } +} + +// TestFallbackRuntimeServers_UsedWhenDiscoveryFailsWithoutCache exercises +// the worst-case path: no cached registry AND no live discovery (embedded +// scenario where AuthTokenFunc returns empty). FallbackServers must still +// surface a usable catalog. +func TestFallbackRuntimeServers_UsedWhenDiscoveryFailsWithoutCache(t *testing.T) { + setEdition(t, &edition.Hooks{ + Name: "wukong", + FallbackServers: func() []edition.ServerInfo { + return []edition.ServerInfo{ + {ID: "conference", Name: "会议", Endpoint: "https://fallback.invalid/conference"}, + } + }, + SupplementServers: func() []edition.ServerInfo { + return []edition.ServerInfo{ + {ID: "extra", Name: "Extra", Endpoint: "https://fallback.invalid/extra"}, + } + }, + }) + + rs := fallbackRuntimeServers() + if len(rs) != 2 { + t.Fatalf("fallbackRuntimeServers() len = %d, want 2 (fallback + non-overlapping supplement); got %v", len(rs), rs) + } + + ids := make(map[string]bool, len(rs)) + for _, r := range rs { + ids[r.Server.CLI.ID] = true + } + for _, want := range []string{"conference", "extra"} { + if !ids[want] { + t.Errorf("fallbackRuntimeServers() missing %q; have %v", want, ids) + } + } +} diff --git a/internal/editionmerge/merge.go b/internal/editionmerge/merge.go new file mode 100644 index 00000000..d7f4f787 --- /dev/null +++ b/internal/editionmerge/merge.go @@ -0,0 +1,87 @@ +// 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 editionmerge converts edition.ServerInfo hooks into +// market.ServerDescriptor values and merges them into discovery results. +// +// This package exists so both internal/cli (runtime catalog loader) and +// internal/app (command-tree loader) can apply the edition's +// SupplementServers / FallbackServers hooks consistently against the same +// discovery pipeline, instead of the hooks being wired only at the +// command-tree layer. Keeping the logic here avoids an import cycle +// between internal/cli ↔ internal/app. +package editionmerge + +import ( + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +// MergeSupplement returns servers augmented with the active edition's +// SupplementServers hook. Discovery entries always win on ID collision — +// the supplement only fills gaps that discovery did not cover. +func MergeSupplement(servers []market.ServerDescriptor) []market.ServerDescriptor { + fn := edition.Get().SupplementServers + if fn == nil { + return servers + } + existing := make(map[string]bool, len(servers)) + for _, s := range servers { + if id := s.CLI.ID; id != "" { + existing[id] = true + } + if s.Key != "" { + existing[s.Key] = true + } + } + for _, sup := range fn() { + if sup.ID == "" || existing[sup.ID] { + continue + } + servers = append(servers, ToDescriptor(sup, "edition_supplement")) + } + return servers +} + +// FallbackToDescriptors converts the edition's FallbackServers hook into +// market.ServerDescriptor values. Callers should only invoke this when +// live discovery returned zero servers and the cache is also empty. +func FallbackToDescriptors(servers []edition.ServerInfo) []market.ServerDescriptor { + out := make([]market.ServerDescriptor, 0, len(servers)) + for _, s := range servers { + out = append(out, ToDescriptor(s, "edition_fallback")) + } + return out +} + +// ToDescriptor is the shared conversion from edition.ServerInfo to the +// market descriptor shape expected by downstream consumers. +// +// Source carries the origin tag for diagnostics / metrics. Supplement and +// fallback entries intentionally carry no ToolOverrides — that keeps +// internal/compat.BuildDynamicCommands from materialising parallel +// command trees for products already owned by hardcoded overlays (see +// internal/compat/dynamic_commands.go's CLIOverlay gate). +func ToDescriptor(s edition.ServerInfo, source string) market.ServerDescriptor { + return market.ServerDescriptor{ + Key: s.ID, + DisplayName: s.Name, + Endpoint: s.Endpoint, + Source: source, + CLI: market.CLIOverlay{ + ID: s.ID, + Command: s.ID, + Prefixes: s.Prefixes, + }, + } +} diff --git a/internal/editionmerge/merge_test.go b/internal/editionmerge/merge_test.go new file mode 100644 index 00000000..ea453aa5 --- /dev/null +++ b/internal/editionmerge/merge_test.go @@ -0,0 +1,123 @@ +// 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 editionmerge + +import ( + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/market" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" +) + +func restoreEdition(t *testing.T) { + t.Helper() + prev := edition.Get() + t.Cleanup(func() { edition.Override(prev) }) +} + +func TestMergeSupplement_DiscoveryWinsOnCollision(t *testing.T) { + restoreEdition(t) + edition.Override(&edition.Hooks{ + SupplementServers: func() []edition.ServerInfo { + return []edition.ServerInfo{ + {ID: "conference", Name: "会议", Endpoint: "https://hardcoded/conference"}, + {ID: "doc", Name: "文档(overridden)", Endpoint: "https://hardcoded/doc"}, + } + }, + }) + + servers := []market.ServerDescriptor{ + { + Key: "doc", + DisplayName: "文档", + Endpoint: "https://live/doc", + CLI: market.CLIOverlay{ID: "doc", Command: "doc"}, + }, + } + + merged := MergeSupplement(servers) + + if len(merged) != 2 { + t.Fatalf("merged len = %d, want 2", len(merged)) + } + + byID := make(map[string]market.ServerDescriptor, len(merged)) + for _, m := range merged { + byID[m.CLI.ID] = m + } + if got := byID["doc"].Endpoint; got != "https://live/doc" { + t.Errorf("doc endpoint = %q, want live endpoint (discovery wins)", got) + } + if got := byID["conference"].Endpoint; got != "https://hardcoded/conference" { + t.Errorf("conference endpoint = %q, want supplement endpoint", got) + } + if got := byID["conference"].Source; got != "edition_supplement" { + t.Errorf("conference Source = %q, want edition_supplement", got) + } +} + +func TestMergeSupplement_NilHookIsNoop(t *testing.T) { + restoreEdition(t) + edition.Override(&edition.Hooks{}) + + servers := []market.ServerDescriptor{ + {Key: "doc", DisplayName: "文档", Endpoint: "https://live/doc", + CLI: market.CLIOverlay{ID: "doc", Command: "doc"}}, + } + + merged := MergeSupplement(servers) + + if len(merged) != 1 { + t.Fatalf("merged len = %d, want 1 (no supplement hook registered)", len(merged)) + } +} + +func TestMergeSupplement_EmptyIDSkipped(t *testing.T) { + restoreEdition(t) + edition.Override(&edition.Hooks{ + SupplementServers: func() []edition.ServerInfo { + return []edition.ServerInfo{ + {ID: "", Name: "empty", Endpoint: "https://example.invalid/empty"}, + {ID: "valid", Name: "valid", Endpoint: "https://example.invalid/valid"}, + } + }, + }) + + merged := MergeSupplement(nil) + if len(merged) != 1 { + t.Fatalf("merged len = %d, want 1 (empty ID must be skipped)", len(merged)) + } + if merged[0].CLI.ID != "valid" { + t.Errorf("merged[0].CLI.ID = %q, want valid", merged[0].CLI.ID) + } +} + +func TestFallbackToDescriptors(t *testing.T) { + got := FallbackToDescriptors([]edition.ServerInfo{ + {ID: "conference", Name: "会议", Endpoint: "https://example.invalid/conference", Prefixes: []string{"conference", "meeting"}}, + }) + if len(got) != 1 { + t.Fatalf("len = %d, want 1", len(got)) + } + g := got[0] + if g.CLI.ID != "conference" || g.CLI.Command != "conference" { + t.Errorf("CLI overlay not wired: %+v", g.CLI) + } + if g.Source != "edition_fallback" { + t.Errorf("Source = %q, want edition_fallback", g.Source) + } + if len(g.CLI.ToolOverrides) != 0 { + t.Errorf("fallback descriptor must not carry ToolOverrides; got %v", g.CLI.ToolOverrides) + } +} diff --git a/pkg/config/constants.go b/pkg/config/constants.go index 39755cb5..df81c2be 100644 --- a/pkg/config/constants.go +++ b/pkg/config/constants.go @@ -74,6 +74,23 @@ const ( DefaultPartition = "default/default" ) +// EditionPartition returns the cache partition for a given edition name. +// The open-source core (name == "" or "open") uses DefaultPartition; every +// other edition gets its own namespace to prevent cross-edition data +// leakage in the disk cache. +// +// This helper takes the edition name as a parameter instead of calling +// edition.Get() so that pkg/config remains a leaf dependency — importable +// from internal/cli, internal/app, internal/cache, etc. without risking +// import cycles. +func EditionPartition(name string) string { + name = strings.TrimSpace(name) + if name == "" || name == "open" { + return DefaultPartition + } + return name + "/default" +} + // ── Auth flow timeouts ────────────────────────────────────────────────── const ( diff --git a/pkg/config/constants_test.go b/pkg/config/constants_test.go index e664810e..b7179f48 100644 --- a/pkg/config/constants_test.go +++ b/pkg/config/constants_test.go @@ -74,6 +74,28 @@ func TestDefaultPartition(t *testing.T) { } } +func TestEditionPartition(t *testing.T) { + t.Parallel() + cases := []struct { + name string + input string + want string + }{ + {"empty maps to default", "", DefaultPartition}, + {"open maps to default", "open", DefaultPartition}, + {"whitespace is trimmed", " wukong ", "wukong/default"}, + {"wukong overlay", "wukong", "wukong/default"}, + {"arbitrary edition", "internal-lab", "internal-lab/default"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := EditionPartition(tc.input); got != tc.want { + t.Fatalf("EditionPartition(%q) = %q, want %q", tc.input, got, tc.want) + } + }) + } +} + func TestManualTokenExpiry(t *testing.T) { t.Parallel() if ManualTokenExpiry <= 0 {