diff --git a/internal/app/legacy.go b/internal/app/legacy.go index 15a603d7..7e711d13 100644 --- a/internal/app/legacy.go +++ b/internal/app/legacy.go @@ -59,25 +59,62 @@ func newLegacyPublicCommands(ctx context.Context, runner executor.Runner) []*cob var loadDynamicCommandsFn = loadDynamicCommands // buildEnvelopeCommandsSafe builds the public command set from the discovery -// envelope, degrading to the hardcoded helper commands when the dynamic -// build panics. +// envelope, self-healing a poisoned cache when the dynamic build panics and +// degrading to the hardcoded helper commands only if that also fails. // // Why this guard exists: the dynamic command tree is constructed from cached // discovery data BEFORE Cobra dispatches any command, so a panic here (e.g. // a duplicate pflag registration fed by a poisoned cache, as seen before // 1.0.32: "chat_permission_grant flag redefined: params") used to abort // every invocation — including `dws cache refresh`, the very command that -// repairs the cache. Recovering locally keeps utility and helper commands -// alive so users can self-heal instead of manually deleting cache files. -func buildEnvelopeCommandsSafe(ctx context.Context, runner executor.Runner) (cmds []*cobra.Command) { +// repairs the cache. +// +// Recovery is two-staged. First the partition's discovery cache is moved +// aside (kept on disk for inspection) and the build retried against a fresh +// fetch — so any path that delivers a fixed binary (`dws upgrade`, reinstall) +// escapes the lock-out with zero manual cache surgery. Only when the rebuild +// panics again (e.g. the remote envelope itself is still poisoned, or the +// machine is offline with no usable cache) does the CLI degrade to utility +// and helper commands with a `dws cache refresh` hint. +func buildEnvelopeCommandsSafe(ctx context.Context, runner executor.Runner) []*cobra.Command { + cmds, panicked := tryBuildEnvelopeCommands(ctx, runner) + if panicked == nil { + return cmds + } + slog.Error("buildEnvelopeCommandsSafe: dynamic command build panicked", "panic", panicked) + + quarantined, qErr := cacheStoreFromEnv().QuarantinePartition(editionPartition()) + if qErr != nil { + slog.Error("buildEnvelopeCommandsSafe: failed to quarantine discovery cache", "error", qErr) + } + if quarantined != "" { + fmt.Fprintf(os.Stderr, + "Warning: building product commands from the local discovery cache failed: %v\n"+ + "The cached discovery data was moved to %s; rebuilding from a fresh fetch...\n", + panicked, quarantined) + cmds, panicked = tryBuildEnvelopeCommands(ctx, runner) + if panicked == nil { + fmt.Fprintln(os.Stderr, "Product commands rebuilt successfully.") + return cmds + } + slog.Error("buildEnvelopeCommandsSafe: rebuild after cache quarantine panicked again, degrading to built-in commands", "panic", panicked) + } + + fmt.Fprintf(os.Stderr, + "Warning: building product commands from the local discovery cache failed: %v\n"+ + "Product commands are temporarily unavailable; utility commands still work.\n"+ + "Run 'dws cache refresh' to rebuild the cache.\n", panicked) + return mergeTopLevelCommands(helpers.NewPublicCommands(runner)) +} + +// tryBuildEnvelopeCommands runs one attempt of the envelope-driven build, +// converting a panic into a return value so the caller can decide between +// self-heal and degradation. +func tryBuildEnvelopeCommands(ctx context.Context, runner executor.Runner) (cmds []*cobra.Command, panicked any) { defer func() { if r := recover(); r != nil { - slog.Error("buildEnvelopeCommandsSafe: dynamic command build panicked, degrading to built-in commands", "panic", r) - fmt.Fprintf(os.Stderr, - "Warning: building product commands from the local discovery cache failed: %v\n"+ - "Product commands are temporarily unavailable; utility commands still work.\n"+ - "Run 'dws cache refresh' to rebuild the cache.\n", r) - cmds = mergeTopLevelCommands(helpers.NewPublicCommands(runner)) + cmds = nil + panicked = r } }() @@ -90,7 +127,7 @@ func buildEnvelopeCommandsSafe(ctx context.Context, runner executor.Runner) (cmd // command surface remains predictable from the envelope alone. helpers.AttachReportLegacyInboxAlias(merged, runner) helpers.AttachReportListReadableEnrichment(merged, runner) - return merged + return merged, nil } // pickCommands returns the union of dynamic and helpers commands. For diff --git a/internal/app/legacy_panic_fallback_test.go b/internal/app/legacy_panic_fallback_test.go index 7524482f..725a4fd8 100644 --- a/internal/app/legacy_panic_fallback_test.go +++ b/internal/app/legacy_panic_fallback_test.go @@ -17,46 +17,163 @@ import ( "context" "io" "os" + "path/filepath" "strings" "testing" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cache" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cli" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" "github.com/spf13/cobra" ) -// TestNewLegacyPublicCommandsPanicFallsBackToHelpers verifies the escape -// hatch for a poisoned discovery cache: when the dynamic command build -// panics (e.g. duplicate pflag registration, the pre-1.0.32 lock-out -// "flag redefined: params"), newLegacyPublicCommands must NOT propagate -// the panic. Instead it degrades to the hardcoded helper commands and -// prints a stderr hint pointing at `dws cache refresh`, so the CLI stays -// usable and the cache can be repaired without manual file deletion. -func TestNewLegacyPublicCommandsPanicFallsBackToHelpers(t *testing.T) { - orig := loadDynamicCommandsFn - loadDynamicCommandsFn = func(context.Context, executor.Runner) []*cobra.Command { - panic("chat_permission_grant flag redefined: params") - } - t.Cleanup(func() { loadDynamicCommandsFn = orig }) - - // Capture stderr to assert the self-heal hint. +// captureStderr redirects os.Stderr for the duration of fn and returns what +// was written to it. +func captureStderr(t *testing.T, fn func()) string { + t.Helper() pipeR, pipeW, err := os.Pipe() if err != nil { t.Fatalf("os.Pipe() error = %v", err) } origStderr := os.Stderr os.Stderr = pipeW - t.Cleanup(func() { os.Stderr = origStderr }) + defer func() { os.Stderr = origStderr }() - cmds := newLegacyPublicCommands(context.Background(), nil) + fn() _ = pipeW.Close() os.Stderr = origStderr captured, _ := io.ReadAll(pipeR) + return string(captured) +} + +// TestNewLegacyPublicCommandsPanicFallsBackToHelpers verifies the escape +// hatch for a poisoned discovery cache: when the dynamic command build +// panics (e.g. duplicate pflag registration, the pre-1.0.32 lock-out +// "flag redefined: params"), newLegacyPublicCommands must NOT propagate +// the panic. With no on-disk cache to quarantine there is nothing to +// self-heal from, so it degrades to the hardcoded helper commands and +// prints a stderr hint pointing at `dws cache refresh`. +func TestNewLegacyPublicCommandsPanicFallsBackToHelpers(t *testing.T) { + t.Setenv(cli.CacheDirEnv, t.TempDir()) + + calls := 0 + orig := loadDynamicCommandsFn + loadDynamicCommandsFn = func(context.Context, executor.Runner) []*cobra.Command { + calls++ + panic("chat_permission_grant flag redefined: params") + } + t.Cleanup(func() { loadDynamicCommandsFn = orig }) + + var cmds []*cobra.Command + captured := captureStderr(t, func() { + cmds = newLegacyPublicCommands(context.Background(), nil) + }) if len(cmds) == 0 { t.Fatalf("newLegacyPublicCommands() = 0 commands after build panic, want helper fallback set") } - if !strings.Contains(string(captured), "dws cache refresh") { + if !strings.Contains(captured, "dws cache refresh") { + t.Errorf("stderr = %q, want a hint mentioning 'dws cache refresh'", captured) + } + if calls != 1 { + t.Errorf("dynamic build attempts = %d, want 1 (no cache on disk, nothing to quarantine and retry)", calls) + } +} + +// TestNewLegacyPublicCommandsSelfHealsPoisonedCache verifies the self-heal +// path: when the build panics AND a discovery cache exists on disk, the +// partition is quarantined (moved aside, kept for inspection) and the build +// retried once. The retry succeeding means the user gets the full dynamic +// command tree with zero manual cache surgery. +func TestNewLegacyPublicCommandsSelfHealsPoisonedCache(t *testing.T) { + tmp := t.TempDir() + t.Setenv(cli.CacheDirEnv, tmp) + + store := cache.NewStore(tmp) + partition := editionPartition() + if err := store.SaveTools(partition, "poisoned-server", cache.ToolsSnapshot{ServerKey: "poisoned-server"}); err != nil { + t.Fatalf("SaveTools() error = %v", err) + } + + calls := 0 + orig := loadDynamicCommandsFn + loadDynamicCommandsFn = func(context.Context, executor.Runner) []*cobra.Command { + calls++ + if calls == 1 { + panic("chat_permission_grant flag redefined: params") + } + return []*cobra.Command{{Use: "dynamic-probe"}} + } + t.Cleanup(func() { loadDynamicCommandsFn = orig }) + + var cmds []*cobra.Command + captured := captureStderr(t, func() { + cmds = newLegacyPublicCommands(context.Background(), nil) + }) + + if calls != 2 { + t.Fatalf("dynamic build attempts = %d, want 2 (initial + retry after quarantine)", calls) + } + found := false + for _, c := range cmds { + if c.Name() == "dynamic-probe" { + found = true + break + } + } + if !found { + t.Errorf("newLegacyPublicCommands() did not return the rebuilt dynamic command tree; got %d commands without 'dynamic-probe'", len(cmds)) + } + + quarantines, _ := filepath.Glob(filepath.Join(tmp, "*.quarantined")) + if len(quarantines) != 1 { + t.Fatalf("quarantine dirs = %v, want exactly 1", quarantines) + } + if _, err := os.Stat(filepath.Join(quarantines[0], "tools", "poisoned-server.json")); err != nil { + t.Errorf("poisoned snapshot not preserved in quarantine: %v", err) + } + if !strings.Contains(captured, "rebuilding from a fresh fetch") { + t.Errorf("stderr = %q, want a note about rebuilding from a fresh fetch", captured) + } + if strings.Contains(captured, "dws cache refresh") { + t.Errorf("stderr = %q, must not tell the user to run 'dws cache refresh' when the rebuild succeeded", captured) + } +} + +// TestNewLegacyPublicCommandsSecondPanicDegradesToHelpers verifies the final +// safety net: if the rebuild after quarantine panics again (remote envelope +// still poisoned, or offline), the CLI degrades to helper commands and keeps +// the `dws cache refresh` hint. +func TestNewLegacyPublicCommandsSecondPanicDegradesToHelpers(t *testing.T) { + tmp := t.TempDir() + t.Setenv(cli.CacheDirEnv, tmp) + + store := cache.NewStore(tmp) + if err := store.SaveTools(editionPartition(), "poisoned-server", cache.ToolsSnapshot{ServerKey: "poisoned-server"}); err != nil { + t.Fatalf("SaveTools() error = %v", err) + } + + calls := 0 + orig := loadDynamicCommandsFn + loadDynamicCommandsFn = func(context.Context, executor.Runner) []*cobra.Command { + calls++ + panic("chat_permission_grant flag redefined: params") + } + t.Cleanup(func() { loadDynamicCommandsFn = orig }) + + var cmds []*cobra.Command + captured := captureStderr(t, func() { + cmds = newLegacyPublicCommands(context.Background(), nil) + }) + + if calls != 2 { + t.Fatalf("dynamic build attempts = %d, want 2 (initial + retry after quarantine)", calls) + } + if len(cmds) == 0 { + t.Fatalf("newLegacyPublicCommands() = 0 commands after repeated build panics, want helper fallback set") + } + if !strings.Contains(captured, "dws cache refresh") { t.Errorf("stderr = %q, want a hint mentioning 'dws cache refresh'", captured) } } diff --git a/internal/app/upgrade.go b/internal/app/upgrade.go index ad7e969b..4bee05eb 100644 --- a/internal/app/upgrade.go +++ b/internal/app/upgrade.go @@ -518,6 +518,16 @@ func runUpgrade(ctx context.Context, opts upgradeOptions) error { fmt.Printf(" %s\n", ugGreen("✓")) } + // Clear discovery-derived caches so the upgraded binary rebuilds its + // command tree from a fresh fetch instead of inheriting snapshots written + // by the old version — a poisoned snapshot used to lock out every + // invocation before the build guards landed (#447 / #449). + if purged, purgeErr := cacheStoreFromEnv().PurgeDiscoveryData(); purgeErr != nil { + fmt.Printf(" %s %s\n", ugYellow("⚠"), ugDim(fmt.Sprintf("清理发现缓存失败 (可手动运行 dws cache refresh): %v", purgeErr))) + } else if len(purged) > 0 { + fmt.Printf(" %s %s\n", ugGreen("✓"), ugDim("发现缓存已清空, 新版本首次运行时自动重建")) + } + // Cleanup old backups rm.Cleanup(5) diff --git a/internal/cache/store.go b/internal/cache/store.go index 56c96554..3fbaa398 100644 --- a/internal/cache/store.go +++ b/internal/cache/store.go @@ -238,6 +238,74 @@ func (s *Store) DeleteDetail(partition, serverKey string) error { return nil } +// QuarantinePartition moves the entire on-disk cache for a partition aside, +// renaming it to ".quarantined", so the next load starts from an +// empty cache while the poisoned snapshot stays on disk for inspection. +// Returns the quarantine path, or "" when the partition has no cache on disk. +// A previous quarantine for the same partition is replaced, so repeated +// quarantines never accumulate. +func (s *Store) QuarantinePartition(partition string) (string, error) { + dir := filepath.Join(s.Root, sanitize(partition)) + if _, err := os.Stat(dir); err != nil { + if os.IsNotExist(err) { + return "", nil + } + return "", err + } + quarantine := dir + ".quarantined" + if err := os.RemoveAll(quarantine); err != nil { + return "", err + } + if err := os.Rename(dir, quarantine); err != nil { + return "", err + } + return quarantine, nil +} + +// discoverySubdirs are the per-partition directories holding discovery-derived +// data: the market registry envelope plus tools / detail snapshots. +var discoverySubdirs = []string{"market", "tools", "detail"} + +// PurgeDiscoveryData deletes the discovery-derived cache for every partition +// under the cache root, leaving unrelated data that shares the root (e.g. the +// upgrade download cache in "downloads/") untouched. Returns the names of the +// partition directories that had data removed. Removal errors are collected +// into the returned error but do not stop the sweep. +func (s *Store) PurgeDiscoveryData() ([]string, error) { + entries, err := os.ReadDir(s.Root) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + var purged []string + var firstErr error + for _, entry := range entries { + if !entry.IsDir() { + continue + } + removedAny := false + for _, sub := range discoverySubdirs { + dir := filepath.Join(s.Root, entry.Name(), sub) + if _, statErr := os.Stat(dir); statErr != nil { + continue + } + if rmErr := os.RemoveAll(dir); rmErr != nil { + if firstErr == nil { + firstErr = rmErr + } + continue + } + removedAny = true + } + if removedAny { + purged = append(purged, entry.Name()) + } + } + return purged, firstErr +} + func (s *Store) registryPath(partition string) string { return filepath.Join(s.Root, sanitize(partition), "market", "servers.json") } diff --git a/internal/cache/store_quarantine_test.go b/internal/cache/store_quarantine_test.go new file mode 100644 index 00000000..b06ef12f --- /dev/null +++ b/internal/cache/store_quarantine_test.go @@ -0,0 +1,131 @@ +// 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 cache + +import ( + "os" + "path/filepath" + "testing" +) + +func TestQuarantinePartitionNoCacheIsNoop(t *testing.T) { + s := NewStore(t.TempDir()) + path, err := s.QuarantinePartition("default_default") + if err != nil { + t.Fatalf("QuarantinePartition() error = %v", err) + } + if path != "" { + t.Errorf("QuarantinePartition() = %q, want empty path when nothing is cached", path) + } +} + +func TestQuarantinePartitionMovesCacheAside(t *testing.T) { + tmp := t.TempDir() + s := NewStore(tmp) + if err := s.SaveTools("default_default", "srv", ToolsSnapshot{ServerKey: "srv"}); err != nil { + t.Fatalf("SaveTools() error = %v", err) + } + + path, err := s.QuarantinePartition("default_default") + if err != nil { + t.Fatalf("QuarantinePartition() error = %v", err) + } + want := filepath.Join(tmp, "default_default.quarantined") + if path != want { + t.Errorf("QuarantinePartition() = %q, want %q", path, want) + } + if _, statErr := os.Stat(filepath.Join(tmp, "default_default")); !os.IsNotExist(statErr) { + t.Errorf("original partition dir still present after quarantine (stat err = %v)", statErr) + } + if _, statErr := os.Stat(filepath.Join(path, "tools", "srv.json")); statErr != nil { + t.Errorf("quarantined snapshot missing: %v", statErr) + } +} + +func TestQuarantinePartitionReplacesPreviousQuarantine(t *testing.T) { + tmp := t.TempDir() + s := NewStore(tmp) + if err := s.SaveTools("default_default", "first", ToolsSnapshot{ServerKey: "first"}); err != nil { + t.Fatalf("SaveTools() error = %v", err) + } + if _, err := s.QuarantinePartition("default_default"); err != nil { + t.Fatalf("first QuarantinePartition() error = %v", err) + } + if err := s.SaveTools("default_default", "second", ToolsSnapshot{ServerKey: "second"}); err != nil { + t.Fatalf("SaveTools() error = %v", err) + } + + path, err := s.QuarantinePartition("default_default") + if err != nil { + t.Fatalf("second QuarantinePartition() error = %v", err) + } + if _, statErr := os.Stat(filepath.Join(path, "tools", "second.json")); statErr != nil { + t.Errorf("latest quarantine missing newest snapshot: %v", statErr) + } + if _, statErr := os.Stat(filepath.Join(path, "tools", "first.json")); !os.IsNotExist(statErr) { + t.Errorf("previous quarantine was not replaced (stat err = %v)", statErr) + } +} + +func TestPurgeDiscoveryDataRemovesDiscoveryDirsOnly(t *testing.T) { + tmp := t.TempDir() + s := NewStore(tmp) + + mustWrite := func(parts ...string) { + t.Helper() + path := filepath.Join(parts...) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll(%s) error = %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil { + t.Fatalf("WriteFile(%s) error = %v", path, err) + } + } + mustWrite(tmp, "default_default", "market", "servers.json") + mustWrite(tmp, "default_default", "tools", "srv.json") + mustWrite(tmp, "default_default", "detail", "srv.json") + mustWrite(tmp, "wukong_default", "tools", "srv.json") + // Unrelated data sharing the cache root must survive the purge. + mustWrite(tmp, "downloads", "dws-1.0.36.tar.gz") + + purged, err := s.PurgeDiscoveryData() + if err != nil { + t.Fatalf("PurgeDiscoveryData() error = %v", err) + } + if len(purged) != 2 { + t.Fatalf("PurgeDiscoveryData() purged = %v, want 2 partitions", purged) + } + for _, sub := range []string{"market", "tools", "detail"} { + if _, statErr := os.Stat(filepath.Join(tmp, "default_default", sub)); !os.IsNotExist(statErr) { + t.Errorf("%s dir survived the purge (stat err = %v)", sub, statErr) + } + } + if _, statErr := os.Stat(filepath.Join(tmp, "wukong_default", "tools")); !os.IsNotExist(statErr) { + t.Errorf("second partition tools dir survived the purge (stat err = %v)", statErr) + } + if _, statErr := os.Stat(filepath.Join(tmp, "downloads", "dws-1.0.36.tar.gz")); statErr != nil { + t.Errorf("unrelated downloads data was removed: %v", statErr) + } +} + +func TestPurgeDiscoveryDataMissingRootIsNoop(t *testing.T) { + s := NewStore(filepath.Join(t.TempDir(), "does-not-exist")) + purged, err := s.PurgeDiscoveryData() + if err != nil { + t.Fatalf("PurgeDiscoveryData() error = %v", err) + } + if len(purged) != 0 { + t.Errorf("PurgeDiscoveryData() purged = %v, want none", purged) + } +}