From e0fd344a26543f54c975bf5c8a308f5a97fd74ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=89=E6=BE=9C?= Date: Tue, 11 Aug 2026 18:13:44 +0800 Subject: [PATCH] fix(skill): always refresh bundled skills --- README.md | 2 +- README_zh.md | 2 +- docs/skill-multi-roadmap.md | 33 +++----- internal/app/skill_setup.go | 23 ++---- internal/app/skill_setup_state_test.go | 7 +- internal/app/upgrade.go | 1 - internal/skillstate/state.go | 92 ++------------------- internal/skillstate/state_test.go | 24 ------ internal/upgrade/paths.go | 108 ++----------------------- internal/upgrade/skill_sync_test.go | 58 ++----------- scripts/install-skills.sh | 14 +++- scripts/install.sh | 14 +++- test/scripts/install_script_test.go | 76 ++++++++++++++++- 13 files changed, 143 insertions(+), 311 deletions(-) diff --git a/README.md b/README.md index 08172915..21534bf9 100644 --- a/README.md +++ b/README.md @@ -439,7 +439,7 @@ DWS_SKILL_SOURCE=/path/to/skills dws skill setup --mode multi > The setup command can remove the opposite-mode layout (`dws/` for multi, `dingtalk-*` for mono) and stale skills not in the bundle. Every removal is previewed before confirmation and preserved under `~/.dws/skill-backups//`; a directory that cannot be backed up is never removed. In a non-interactive shell, first run `--dry-run` and inspect its output; only then may the caller explicitly choose the scripting-only confirmation bypass. -After a successful multi setup, DWS stores the official bundle snapshot in `~/.dws/skills-state.json` (or `$DWS_CONFIG_DIR/skills-state.json`). A normal `dws upgrade` refreshes official skills that are still installed, automatically adds skills introduced by the new release, and leaves older official skills that you deleted locally absent. `dws upgrade --force` restores the complete official set. A missing/unreadable state file or an empty local set falls back to a full install, matching a first-time update. +After a multi setup or upgrade, DWS stores an informational official bundle snapshot in `~/.dws/skills-state.json` (or `$DWS_CONFIG_DIR/skills-state.json`). Every upgrade installs and overwrites the complete bundled Skill set from that release. Deleting or excluding a bundled Skill is not sticky: the next upgrade restores it. `dws upgrade --force` additionally allows reinstalling the current CLI version when no newer version is available. Env vars: `DWS_SKILL_MODE=mono|multi` (also honored by `install.sh` / `install.ps1`), `DWS_SKILL_SOURCE=`. diff --git a/README_zh.md b/README_zh.md index 509e708b..6fe9cbe4 100644 --- a/README_zh.md +++ b/README_zh.md @@ -433,7 +433,7 @@ DWS_SKILL_SOURCE=/path/to/skills dws skill setup --mode multi > setup 命令可能移除对面模式残留（装 multi 删 `dws/`，装 mono 删 `dingtalk-*`）以及不在 bundle 内的过期 skill。所有删除都会先列入确认预览，并备份到 `~/.dws/skill-backups/<时间戳>/`；备份失败的目录会保留原样、绝不删除。非交互环境应先用 `--dry-run` 核对输出，再由调用方显式决定是否使用仅供脚本的确认跳过参数。 -multi setup 成功后，DWS 会把官方 bundle 快照写入 `~/.dws/skills-state.json`（或 `$DWS_CONFIG_DIR/skills-state.json`）。普通 `dws upgrade` 只刷新仍在本地的官方 Skill，自动加入新版新增 Skill，但不会装回用户已删除的旧官方 Skill。`dws upgrade --force` 恢复官方全量集合。状态文件缺失/不可读或本地集合为空时，按首次更新处理，回退为全量安装。 +multi setup 或 upgrade 后，DWS 会把官方 bundle 快照作为信息记录写入 `~/.dws/skills-state.json`（或 `$DWS_CONFIG_DIR/skills-state.json`）。每次 upgrade 都会安装并覆盖该版本的全部预制 Skill；手工删除或通过 setup 排除预制 Skill 不会永久保留，下次 upgrade 会恢复。`dws upgrade --force` 还允许在没有新版本时重装当前 CLI 版本。 环境变量：`DWS_SKILL_MODE=mono|multi`（`install.sh` / `install.ps1` 也认）、`DWS_SKILL_SOURCE=<路径>`。 diff --git a/docs/skill-multi-roadmap.md b/docs/skill-multi-roadmap.md index fac4ca32..d6940997 100644 --- a/docs/skill-multi-roadmap.md +++ b/docs/skill-multi-roadmap.md @@ -10,11 +10,11 @@ | 项 | 内容 | |---|---| -| 当前阶段 | 默认 multi、安全迁移、官方 Skill 增量升级已落地；独立 `dws skill mode` lifecycle 命令不建设 | +| 当前阶段 | 默认 multi、安全迁移、官方 Skill 全量覆盖升级已落地；独立 `dws skill mode` lifecycle 命令不建设 | | 硬 deadline | 2026-08-30：安装/升级默认 multi；mono 仅 opt-in；物理下线在独立 retirement 版本推进 | -| 已完成 | 五面默认 multi、互斥清理、删除前备份、setup 确认/`--dry-run`、官方清单快照与增量升级 | +| 已完成 | 五面默认 multi、互斥清理、删除前备份、setup 确认/`--dry-run`、官方清单全量覆盖升级 | | 下一步 | beta/L1 观察、可选 agent-home 清单门禁、mono retirement | -| 终态 | multi 为唯一默认布局；保留明确的用户删除选择，新版官方 Skill 可自动加入 | +| 终态 | multi 为唯一默认布局；每次升级全量覆盖预制 Skill，不持久化本地删除选择 | ## 1. 最终语义 @@ -34,25 +34,18 @@ - `LocateSkillsRoot` 优先返回 release zip 内的 `multi/`。包中存在 multi 树时， 存量 mono 布局会迁移为 multi；仅 legacy mono-only 包走 mono 回退路径。 -- 布局由包内容驱动，不写 mode sticky 状态；Skill 集合则按本地存在性和官方 - 快照增量求解： - - ```text - (本地仍安装 ∩ 本版官方清单) ∪ (本版官方清单 - 上版官方清单) - ``` - -- 因此，本地删除的旧官方 Skill 不会在普通升级中被装回，新版新增官方 Skill - 会自动加入；`dws upgrade --force` 恢复本版官方全量集合。 -- `dingtalk-shared` 是强制依赖，只要 bundle 提供就始终包含。 -- 状态文件缺失/不可读、本地官方集合为空，或本地扫描失败时按首次升级处理， - 回退为全量安装。 +- 布局和 Skill 集合都由当前升级包驱动，不写 mode sticky 状态，也不根据本地 + 目录缺失推断排除意图。每次 upgrade 都安装并覆盖本版官方全量集合。 +- 因此，本地删除或通过 setup 临时排除的预制 Skill 会在下一次 upgrade 恢复， + 新版新增官方 Skill 也会自动加入；`--force` 主要用于重装当前 CLI 版本。 +- `dingtalk-shared` 随官方全量集合始终安装。 ### 1.3 状态与缓存 -- multi setup/upgrade 成功后写入 `~/.dws/skills-state.json`；设置 +- multi setup/upgrade 成功后写入信息快照 `~/.dws/skills-state.json`；设置 `DWS_CONFIG_DIR` 时写到该目录下的 `skills-state.json`。 -- 状态记录当前版本官方清单、本次更新集合、新增官方 Skill、跳过的本地删除 - Skill 和更新时间。它只用于增量求集合，不决定 mono/multi 布局。 +- 状态记录当前版本官方清单、本次更新集合和更新时间，不参与安装集合求解， + 也不决定 mono/multi 布局。 - `~/.dws/skills/{mono,multi}` 是 setup 的本地回退源。所有渠道先在缓存同级 staging 目录完成复制，再通过 rename 发布；复制或发布失败时保留/恢复旧缓存。 @@ -98,7 +91,7 @@ Homebrew 不直接铺 Agent home，安装后由 `dws skill setup` 完成布局 - 默认 multi 和五面安装入口对齐。 - mono/multi 互斥、过期目录与同名目录删除前备份。 - setup 精确计划、`--dry-run`、非交互确认门禁。 -- 官方清单状态文件与普通/`--force` 增量升级语义。 +- 官方清单信息快照与普通/`--force` 全量覆盖升级语义。 - 缓存 staged publish 与旧缓存恢复。 ### 后续可选 @@ -121,7 +114,7 @@ Homebrew 不直接铺 Agent home，安装后由 `dws skill setup` 完成布局 |---|---| | 多目标复制中断 | 旧目录保存在 `skill-backups`，目标报告失败；后续可补自动 rollback UX | | mono/multi 漂移 | 安装互斥清理；备份失败时不铺相反布局；upgrade 有 multi 包时迁移布局 | -| 用户删除的 Skill 被装回 | 官方快照增量公式；仅 `--force` 恢复全量 | +| 预制 Skill 被用户删除或修改 | 下一次 upgrade 从官方 bundle 全量恢复并覆盖 | | 状态文件损坏 | 明确回退全量并重写有效状态，不从目录猜测 CLI 参数意图 | | 缓存复制失败 | 同级 staging 完整复制后发布；失败保留或恢复旧缓存 | | 无服务端灰度 | beta 轨先行、issue/回访观察、必要时撤回 release 并引导重装 mono | diff --git a/internal/app/skill_setup.go b/internal/app/skill_setup.go index a4b4687f..ccbb65e7 100644 --- a/internal/app/skill_setup.go +++ b/internal/app/skill_setup.go @@ -270,7 +270,7 @@ func runSkillSetup(cmd *cobra.Command, _ []string) error { return err } if installed > 0 && skipped > 0 { - fmt.Fprintln(errOut, "⚠️ 本次 Skill 安装不完整，保留原更新状态；修复失败原因后请重试 setup 或使用 upgrade --force") + fmt.Fprintln(errOut, "⚠️ 本次 Skill 安装不完整；修复失败原因后请重试 setup，或运行普通 upgrade 全量刷新预制 Skill") } if installed > 0 && skipped == 0 { home, homeErr := skillSetupUserHomeDir() @@ -283,25 +283,14 @@ func runSkillSetup(cmd *cobra.Command, _ []string) error { updatedSkillNames = append(updatedSkillNames, multiMiscSkill) sort.Strings(updatedSkillNames) } - selected := make(map[string]bool, len(updatedSkillNames)) - for _, name := range updatedSkillNames { - selected[name] = true - } - var stateSkipped []string - for _, name := range allMultiSkillNames { - if !selected[name] { - stateSkipped = append(stateSkipped, name) - } - } state := skillstate.State{ - Version: RawVersion(), - OfficialSkills: allMultiSkillNames, - UpdatedSkills: updatedSkillNames, - SkippedDeletedSkills: stateSkipped, - UpdatedAt: skillSetupNow().UTC().Format(time.RFC3339), + Version: RawVersion(), + OfficialSkills: allMultiSkillNames, + UpdatedSkills: updatedSkillNames, + UpdatedAt: skillSetupNow().UTC().Format(time.RFC3339), } if stateErr := skillSetupWriteState(home, state); stateErr != nil { - return fmt.Errorf("Skill 已安装，但保存后续增量更新状态失败: %w", stateErr) + return fmt.Errorf("Skill 已安装，但保存官方 Skill 信息快照失败: %w", stateErr) } } else if stateErr := skillSetupRemoveState(home); stateErr != nil { return fmt.Errorf("mono Skill 已安装，但清理 multi 更新状态失败: %w", stateErr) diff --git a/internal/app/skill_setup_state_test.go b/internal/app/skill_setup_state_test.go index e7605be2..e341e16e 100644 --- a/internal/app/skill_setup_state_test.go +++ b/internal/app/skill_setup_state_test.go @@ -13,7 +13,7 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" ) -func TestCrossPlatformCoverageSkillSetupPersistsLarkStyleBaseline(t *testing.T) { +func TestCrossPlatformCoverageSkillSetupPersistsOfficialSnapshot(t *testing.T) { home := t.TempDir() testseam.Swap(t, &skillSetupResolveMode, func(mode string, _ bool, _ io.Writer) (string, error) { return mode, nil }) testseam.Swap(t, &skillSetupResolveSource, func(string, string) (string, func(), error) { return "source", func() {}, nil }) @@ -46,14 +46,13 @@ func TestCrossPlatformCoverageSkillSetupPersistsLarkStyleBaseline(t *testing.T) t.Fatal(err) } if !reflect.DeepEqual(saved.OfficialSkills, []string{"dingtalk-a", "dingtalk-b", "dingtalk-shared"}) || - !reflect.DeepEqual(saved.UpdatedSkills, []string{"dingtalk-shared", "dingtalk-a"}) || - !reflect.DeepEqual(saved.SkippedDeletedSkills, []string{"dingtalk-b"}) { + !reflect.DeepEqual(saved.UpdatedSkills, []string{"dingtalk-shared", "dingtalk-a"}) { t.Fatalf("saved = %#v", saved) } testseam.Swap(t, &skillSetupWriteState, func(string, skillstate.State) error { return errors.New("denied") }) cmd = skillSetupCoverageCommand(t, skillSetupModeMulti, true) - if err := cmd.RunE(cmd, nil); err == nil || !strings.Contains(err.Error(), "状态失败") { + if err := cmd.RunE(cmd, nil); err == nil || !strings.Contains(err.Error(), "信息快照失败") { t.Fatalf("write-state error = %v", err) } diff --git a/internal/app/upgrade.go b/internal/app/upgrade.go index daaf4b0f..b9099dd7 100644 --- a/internal/app/upgrade.go +++ b/internal/app/upgrade.go @@ -598,7 +598,6 @@ func runUpgrade(ctx context.Context, opts upgradeOptions) error { if hasSkills { result, installErr := installUpgradeSkills(skillSrc, upgrade.SkillUpgradeOptions{ - Force: opts.force, Version: release.Version, }) if installErr != nil { diff --git a/internal/skillstate/state.go b/internal/skillstate/state.go index 439de9f0..4c454adc 100644 --- a/internal/skillstate/state.go +++ b/internal/skillstate/state.go @@ -1,9 +1,9 @@ // Copyright 2026 Alibaba Group // Licensed under the Apache License, Version 2.0 -// Package skillstate persists the official Skill snapshot used by incremental -// updates. Its model follows lark-cli: local presence expresses continued use, -// while additions are derived by comparing consecutive official snapshots. +// Package skillstate persists the official Skill snapshot written after setup +// and upgrade. The snapshot is informational; bundled skills are always fully +// refreshed from the current release and local absence is not an exclusion. package skillstate import ( @@ -25,27 +25,10 @@ var ( ) type State struct { - Version string `json:"version"` - OfficialSkills []string `json:"official_skills"` - UpdatedSkills []string `json:"updated_skills"` - AddedOfficialSkills []string `json:"added_official_skills"` - SkippedDeletedSkills []string `json:"skipped_deleted_skills"` - UpdatedAt string `json:"updated_at"` -} - -type SyncInput struct { - OfficialSkills []string - LocalSkills []string - PreviousState *State - StateReadable bool - Force bool -} - -type SyncPlan struct { - OfficialSkills []string - ToUpdate []string - Added []string - SkippedDeleted []string + Version string `json:"version"` + OfficialSkills []string `json:"official_skills"` + UpdatedSkills []string `json:"updated_skills"` + UpdatedAt string `json:"updated_at"` } func Path(home string) string { @@ -73,8 +56,6 @@ func Read(home string) (*State, bool, error) { func Write(home string, state State) error { state.OfficialSkills = uniqueSorted(state.OfficialSkills) state.UpdatedSkills = uniqueSorted(state.UpdatedSkills) - state.AddedOfficialSkills = uniqueSorted(state.AddedOfficialSkills) - state.SkippedDeletedSkills = uniqueSorted(state.SkippedDeletedSkills) data, _ := json.MarshalIndent(state, "", " ") if err := helpers.AtomicWriteJSON(Path(home), append(data, '\n')); err != nil { return fmt.Errorf("保存 Skill 状态失败: %w", err) @@ -89,48 +70,6 @@ func Remove(home string) error { return nil } -// Plan applies the lark-cli reconciliation formula: -// -// (local installed ∩ current official) ∪ (current official - previous official) -func Plan(input SyncInput) SyncPlan { - official := uniqueSorted(input.OfficialSkills) - if input.Force { - return SyncPlan{OfficialSkills: official, ToUpdate: official} - } - officialSet := toSet(official) - updateSet := toSet(intersection(input.LocalSkills, officialSet)) - previousSet := map[string]bool{} - if input.StateReadable && input.PreviousState != nil { - previousSet = toSet(input.PreviousState.OfficialSkills) - } - var added []string - for _, name := range official { - if !previousSet[name] { - added = append(added, name) - updateSet[name] = true - } - } - toUpdate := sortedKeys(updateSet) - updatedSet := toSet(toUpdate) - var skipped []string - for _, name := range official { - if !updatedSet[name] { - skipped = append(skipped, name) - } - } - return SyncPlan{OfficialSkills: official, ToUpdate: toUpdate, Added: uniqueSorted(added), SkippedDeleted: skipped} -} - -func intersection(values []string, allowed map[string]bool) []string { - var out []string - for _, value := range uniqueSorted(values) { - if allowed[value] { - out = append(out, value) - } - } - return out -} - func uniqueSorted(values []string) []string { seen := make(map[string]bool, len(values)) out := make([]string, 0, len(values)) @@ -144,20 +83,3 @@ func uniqueSorted(values []string) []string { sort.Strings(out) return out } - -func toSet(values []string) map[string]bool { - out := make(map[string]bool, len(values)) - for _, value := range values { - out[value] = true - } - return out -} - -func sortedKeys(values map[string]bool) []string { - out := make([]string, 0, len(values)) - for value := range values { - out = append(out, value) - } - sort.Strings(out) - return out -} diff --git a/internal/skillstate/state_test.go b/internal/skillstate/state_test.go index a98e5a05..d8da9183 100644 --- a/internal/skillstate/state_test.go +++ b/internal/skillstate/state_test.go @@ -11,30 +11,6 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" ) -func TestCrossPlatformCoverageSkillStatePlanMatchesLarkIncrementalModel(t *testing.T) { - previous := &State{OfficialSkills: []string{"dingtalk-a", "dingtalk-b"}} - plan := Plan(SyncInput{ - OfficialSkills: []string{"dingtalk-a", "dingtalk-b", "dingtalk-c", "dingtalk-c", ""}, - LocalSkills: []string{"dingtalk-a", "custom", "dingtalk-a"}, - PreviousState: previous, - StateReadable: true, - }) - if want := []string{"dingtalk-a", "dingtalk-c"}; !reflect.DeepEqual(plan.ToUpdate, want) { - t.Fatalf("ToUpdate = %v, want %v", plan.ToUpdate, want) - } - if !reflect.DeepEqual(plan.Added, []string{"dingtalk-c"}) || !reflect.DeepEqual(plan.SkippedDeleted, []string{"dingtalk-b"}) { - t.Fatalf("plan = %#v", plan) - } - force := Plan(SyncInput{OfficialSkills: []string{"dingtalk-b", "dingtalk-a"}, Force: true}) - if !reflect.DeepEqual(force.ToUpdate, []string{"dingtalk-a", "dingtalk-b"}) { - t.Fatalf("force = %#v", force) - } - cold := Plan(SyncInput{OfficialSkills: []string{"dingtalk-a", "dingtalk-b"}}) - if !reflect.DeepEqual(cold.ToUpdate, cold.OfficialSkills) { - t.Fatalf("cold = %#v", cold) - } -} - func TestCrossPlatformCoverageSkillStateReadWriteRemoveAndErrors(t *testing.T) { t.Setenv("DWS_CONFIG_DIR", "") home := t.TempDir() diff --git a/internal/upgrade/paths.go b/internal/upgrade/paths.go index fcda6d42..e8c96ef4 100644 --- a/internal/upgrade/paths.go +++ b/internal/upgrade/paths.go @@ -66,7 +66,6 @@ var ( upgradeReadDir = os.ReadDir upgradeStat = os.Stat upgradeBackupStamp = func() string { return time.Now().UTC().Format("20060102-150405") } - upgradeReadSkillState = skillstate.Read upgradeWriteSkillState = skillstate.Write upgradeNow = time.Now ) @@ -188,10 +187,9 @@ func (r *SkillUpgradeResult) Failed() []SkillDirResult { // preferred when present). // // Package-driven layout: -// - release zip has multi/ → reconcile the local official skill set with -// the previous official snapshot. Installed official skills are refreshed, -// newly introduced official skills are added, and locally deleted older -// skills remain absent. --force restores the full official set. +// - release zip has multi/ → install and overwrite the complete official +// bundle. Locally deleted bundled skills are restored on the next upgrade; +// local absence is never treated as a persistent exclusion. // - dingtalk-shared is mandatory whenever it exists in the bundle. // - legacy zip with no multi tree → mono refresh path (unchanged fallback) // @@ -215,7 +213,6 @@ func UpgradeSkillLocations(extractedDir string) (*SkillUpgradeResult, error) { } type SkillUpgradeOptions struct { - Force bool Version string } @@ -228,37 +225,16 @@ func UpgradeSkillLocationsWithOptions(extractedDir string, opts SkillUpgradeOpti multiRoot, skills := resolveMultiBundle(extractedDir) if len(skills) > 0 { official := append([]string(nil), skills...) - previous, readable, stateErr := upgradeReadSkillState(homeDir) - if stateErr != nil { - previous, readable = nil, false - } - local, localErr := listInstalledOfficialSkills(homeDir, official) - plan := skillstate.Plan(skillstate.SyncInput{ - OfficialSkills: official, - LocalSkills: local, - PreviousState: previous, - StateReadable: readable, - Force: opts.Force, - }) - if localErr != nil || len(local) == 0 || len(plan.ToUpdate) == 0 { - plan.ToUpdate = official - plan.Added = official - plan.SkippedDeleted = nil - } - plan.ToUpdate = ensureMandatoryUpgradeShared(plan.ToUpdate, official) - plan.SkippedDeleted = skippedOfficialSkills(official, plan.ToUpdate) - result, installErr := upgradeMultiSkillLocations(homeDir, multiRoot, plan.ToUpdate) + result, installErr := upgradeMultiSkillLocations(homeDir, multiRoot, official) if installErr != nil { return result, installErr } if len(result.Failed()) == 0 && len(result.Succeeded()) > 0 { state := skillstate.State{ - Version: opts.Version, - OfficialSkills: official, - UpdatedSkills: plan.ToUpdate, - AddedOfficialSkills: plan.Added, - SkippedDeletedSkills: plan.SkippedDeleted, - UpdatedAt: upgradeNow().UTC().Format(time.RFC3339), + Version: opts.Version, + OfficialSkills: official, + UpdatedSkills: official, + UpdatedAt: upgradeNow().UTC().Format(time.RFC3339), } if writeErr := upgradeWriteSkillState(homeDir, state); writeErr != nil { return result, fmt.Errorf("Skill 已同步但状态未写入: %w", writeErr) @@ -273,74 +249,6 @@ func UpgradeSkillLocationsWithOptions(extractedDir string, opts SkillUpgradeOpti return nil, fmt.Errorf("升级包中找不到可安装的 skill 源") } -func listInstalledOfficialSkills(homeDir string, official []string) ([]string, error) { - officialSet := make(map[string]bool, len(official)) - for _, name := range official { - officialSet[name] = true - } - installed := map[string]bool{} - for _, agentDir := range knownSkillDirs { - base := filepath.Join(homeDir, agentDir) - entries, err := upgradeReadDir(base) - if err != nil { - if os.IsNotExist(err) { - continue - } - return nil, err - } - for _, entry := range entries { - if !entry.IsDir() || !officialSet[entry.Name()] { - continue - } - if info, statErr := upgradeStat(filepath.Join(base, entry.Name(), "SKILL.md")); statErr == nil && !info.IsDir() { - installed[entry.Name()] = true - } - } - } - names := make([]string, 0, len(installed)) - for name := range installed { - names = append(names, name) - } - sort.Strings(names) - return names, nil -} - -func ensureMandatoryUpgradeShared(selected, official []string) []string { - hasShared, selectedShared := false, false - for _, name := range official { - if name == "dingtalk-shared" { - hasShared = true - break - } - } - for _, name := range selected { - if name == "dingtalk-shared" { - selectedShared = true - break - } - } - if hasShared && !selectedShared { - selected = append(selected, "dingtalk-shared") - sort.Strings(selected) - } - return selected -} - -func skippedOfficialSkills(official, selected []string) []string { - selectedSet := make(map[string]bool, len(selected)) - for _, name := range selected { - selectedSet[name] = true - } - var skipped []string - for _, name := range official { - if !selectedSet[name] { - skipped = append(skipped, name) - } - } - sort.Strings(skipped) - return skipped -} - // skillBackupKeep limits ~/.dws/skill-backups/ growth: only the newest // backups are kept. const skillBackupKeep = 5 diff --git a/internal/upgrade/skill_sync_test.go b/internal/upgrade/skill_sync_test.go index e6cf6676..8f25e571 100644 --- a/internal/upgrade/skill_sync_test.go +++ b/internal/upgrade/skill_sync_test.go @@ -13,7 +13,7 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" ) -func TestCrossPlatformCoverageIncrementalSkillUpgradeMatchesLarkModel(t *testing.T) { +func TestCrossPlatformCoverageSkillUpgradeAlwaysRestoresOfficialBundle(t *testing.T) { t.Setenv("DWS_CONFIG_DIR", "") home := withFakeHome(t) testseam.Swap(t, &knownSkillDirs, []string{".agents/skills"}) @@ -33,30 +33,21 @@ func TestCrossPlatformCoverageIncrementalSkillUpgradeMatchesLarkModel(t *testing multi := writeMultiBundle(t, t.TempDir(), "dingtalk-a", "dingtalk-b", "dingtalk-c", "dingtalk-shared") result, err := UpgradeSkillLocationsWithOptions(multi, SkillUpgradeOptions{Version: "1.1.0"}) if err != nil || len(result.Failed()) != 0 { - t.Fatalf("incremental = %#v, %v", result, err) + t.Fatalf("full refresh = %#v, %v", result, err) } - if _, err := os.Stat(filepath.Join(base, "dingtalk-b")); !os.IsNotExist(err) { - t.Fatalf("deleted old Skill restored: %v", err) - } - for _, name := range []string{"dingtalk-a", "dingtalk-c", "dingtalk-shared"} { + for _, name := range []string{"dingtalk-a", "dingtalk-b", "dingtalk-c", "dingtalk-shared"} { if _, err := os.Stat(filepath.Join(base, name, "SKILL.md")); err != nil { - t.Fatal(err) + t.Fatalf("official Skill %s was not restored: %v", name, err) } } state, readable, err := skillstate.Read(home) - if err != nil || !readable || !reflect.DeepEqual(state.AddedOfficialSkills, []string{"dingtalk-c"}) || !reflect.DeepEqual(state.SkippedDeletedSkills, []string{"dingtalk-b"}) { + wantOfficial := []string{"dingtalk-a", "dingtalk-b", "dingtalk-c", "dingtalk-shared"} + if err != nil || !readable || !reflect.DeepEqual(state.OfficialSkills, wantOfficial) || !reflect.DeepEqual(state.UpdatedSkills, wantOfficial) { t.Fatalf("state = %#v, %v, %v", state, readable, err) } - result, err = UpgradeSkillLocationsWithOptions(multi, SkillUpgradeOptions{Force: true}) - if err != nil || len(result.Failed()) != 0 { - t.Fatalf("force = %#v, %v", result, err) - } - if _, err := os.Stat(filepath.Join(base, "dingtalk-b", "SKILL.md")); err != nil { - t.Fatalf("force restore: %v", err) - } } -func TestCrossPlatformCoverageIncrementalSkillUpgradeFallbacksAndHelpers(t *testing.T) { +func TestCrossPlatformCoverageFullSkillUpgradeIgnoresOldStateAndReportsWriteFailure(t *testing.T) { t.Setenv("DWS_CONFIG_DIR", "") testseam.Swap(t, &knownSkillDirs, []string{".agents/skills"}) multi := writeMultiBundle(t, t.TempDir(), "dingtalk-a", "dingtalk-b", "dingtalk-shared") @@ -70,7 +61,7 @@ func TestCrossPlatformCoverageIncrementalSkillUpgradeFallbacksAndHelpers(t *test } result, err := UpgradeSkillLocationsWithOptions(multi, SkillUpgradeOptions{}) if err != nil || len(result.Succeeded()) != 1 { - t.Fatalf("cold fallback = %#v, %v", result, err) + t.Fatalf("corrupt old state should not block full refresh = %#v, %v", result, err) } home2 := t.TempDir() testseam.Swap(t, &upgradeUserHomeDir, func() (string, error) { return home2, nil }) @@ -79,37 +70,4 @@ func TestCrossPlatformCoverageIncrementalSkillUpgradeFallbacksAndHelpers(t *test if err == nil || !strings.Contains(err.Error(), "状态未写入") || len(result.Succeeded()) != 1 { t.Fatalf("write failure = %#v, %v", result, err) } - home3 := t.TempDir() - base := filepath.Join(home3, ".agents", "skills") - if err := os.MkdirAll(filepath.Join(base, "dingtalk-a", "SKILL.md"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(base, "dingtalk-b"), []byte("file"), 0o600); err != nil { - t.Fatal(err) - } - if got, err := listInstalledOfficialSkills(home3, []string{"dingtalk-a", "dingtalk-b"}); err != nil || len(got) != 0 { - t.Fatalf("filtered local = %v, %v", got, err) - } - originalReadDir := upgradeReadDir - testseam.Swap(t, &upgradeReadDir, func(path string) ([]os.DirEntry, error) { - if path == base { - return nil, errors.New("read denied") - } - return originalReadDir(path) - }) - if _, err := listInstalledOfficialSkills(home3, []string{"dingtalk-a"}); err == nil { - t.Fatal("blocked local discovery succeeded") - } - if got := ensureMandatoryUpgradeShared([]string{"dingtalk-a"}, []string{"dingtalk-a"}); !reflect.DeepEqual(got, []string{"dingtalk-a"}) { - t.Fatal(got) - } - if got := ensureMandatoryUpgradeShared([]string{"dingtalk-a", "dingtalk-shared"}, []string{"dingtalk-shared"}); !reflect.DeepEqual(got, []string{"dingtalk-a", "dingtalk-shared"}) { - t.Fatal(got) - } - if got := ensureMandatoryUpgradeShared([]string{"dingtalk-a"}, []string{"dingtalk-a", "dingtalk-shared"}); !reflect.DeepEqual(got, []string{"dingtalk-a", "dingtalk-shared"}) { - t.Fatal(got) - } - if got := skippedOfficialSkills([]string{"dingtalk-shared", "dingtalk-b", "dingtalk-a"}, []string{"dingtalk-shared", "dingtalk-a"}); !reflect.DeepEqual(got, []string{"dingtalk-b"}) { - t.Fatal(got) - } } diff --git a/scripts/install-skills.sh b/scripts/install-skills.sh index c7b1a7ee..6acedd05 100755 --- a/scripts/install-skills.sh +++ b/scripts/install-skills.sh @@ -268,6 +268,7 @@ install_multi_skills_to_root() { root="$2" installed=0 attempted=0 + failed=0 idx=0 for agent_dir in \ ".agents/skills" \ @@ -297,7 +298,8 @@ install_multi_skills_to_root() { if _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir"; then installed=$((installed + 1)) else - printf ' ⚠️ 跳过 %s（备份失败，未安装 multi）\n' "$base_dir" + failed=$((failed + 1)) + printf ' ⚠️ 跳过 %s（备份或复制失败，未完成 multi 安装）\n' "$base_dir" fi idx=$((idx + 1)) done @@ -306,6 +308,11 @@ install_multi_skills_to_root() { fi if [ "$installed" -eq 0 ]; then printf ' ⚠️ 未安装任何 multi Skill：所有检测到的 Agent 目标均失败\n' + return 1 + fi + if [ "$failed" -gt 0 ]; then + printf ' ⚠️ 有 %s 个 Agent 目标安装失败\n' "$failed" + return 1 fi } @@ -348,7 +355,10 @@ _install_multi_to_base() { _name="$(basename "$skill_dir")" _dest="$_base/$_name" mkdir -p "$_dest" || return 1 - cp -R "${skill_dir}." "$_dest/" 2>/dev/null || cp -r "${skill_dir}." "$_dest/" + if ! cp -R "${skill_dir}." "$_dest/" 2>/dev/null && ! cp -r "${skill_dir}." "$_dest/"; then + printf ' ⚠️ Skill 复制失败，目标未计为安装成功: %s\n' "$_dest" + return 1 + fi _count=$((_count + 1)) done diff --git a/scripts/install.sh b/scripts/install.sh index f92ac37b..98890b93 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -561,6 +561,7 @@ install_multi_skills_to_homes() { root="${HOME}" installed=0 attempted=0 + failed=0 idx=0 for agent_dir in \ ".agents/skills" \ @@ -590,7 +591,8 @@ install_multi_skills_to_homes() { if _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir"; then installed=$((installed + 1)) else - say " ⚠️ 跳过 ${base_dir}（备份失败，未安装 multi）" + failed=$((failed + 1)) + say " ⚠️ 跳过 ${base_dir}（备份或复制失败，未完成 multi 安装）" fi idx=$((idx + 1)) done @@ -599,6 +601,11 @@ install_multi_skills_to_homes() { fi if [ "$installed" -eq 0 ]; then say " ⚠️ 未安装任何 multi Skill：所有检测到的 Agent 目标均失败" + return 1 + fi + if [ "$failed" -gt 0 ]; then + say " ⚠️ 有 ${failed} 个 Agent 目标安装失败" + return 1 fi } @@ -641,7 +648,10 @@ _install_multi_to_base() { _name="$(basename "$skill_dir")" _dest="$_base/$_name" mkdir -p "$_dest" || return 1 - cp -R "${skill_dir}." "$_dest/" 2>/dev/null || cp -r "${skill_dir}." "$_dest/" + if ! cp -R "${skill_dir}." "$_dest/" 2>/dev/null && ! cp -r "${skill_dir}." "$_dest/"; then + say " ⚠️ Skill 复制失败，目标未计为安装成功: $_dest" + return 1 + fi _count=$((_count + 1)) done diff --git a/test/scripts/install_script_test.go b/test/scripts/install_script_test.go index f3840130..4a1ded01 100644 --- a/test/scripts/install_script_test.go +++ b/test/scripts/install_script_test.go @@ -1073,10 +1073,16 @@ func TestInstallScriptBackupFailureKeepsOriginalDir(t *testing.T) { t.Fatalf("mono run must keep the original dws/ untouched on backup failure (data=%q, err=%v)\noutput:\n%s", string(data), err, out) } - out = runInstallScript(t, fixture.scriptPath, fixture.envWithSkillMode("multi", + cmd := exec.Command("sh", fixture.scriptPath) + cmd.Env = fixture.envWithSkillMode("multi", "DWS_INSTALL_DIR="+installDir, "DWS_NO_SKILLS=0", - )) + ) + output, runErr := cmd.CombinedOutput() + if runErr == nil { + t.Fatalf("multi install unexpectedly succeeded after backup failure:\n%s", output) + } + out = string(output) if !strings.Contains(out, "保留原目录") { t.Fatalf("expected backup-failure warning in multi output:\n%s", out) } @@ -1125,8 +1131,12 @@ install_multi_skills_to_root "$DWS_TEST_MULTI" "$DWS_TEST_ROOT" "DWS_TEST_MULTI="+multi, "DWS_TEST_ROOT="+root, ) - if output, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("install-skills harness failed: %v\n%s", err, output) + output, runErr := cmd.CombinedOutput() + if runErr == nil { + t.Fatalf("install-skills harness unexpectedly succeeded after backup failure:\n%s", output) + } + if !strings.Contains(string(output), "所有检测到的 Agent 目标均失败") { + t.Fatalf("install-skills aggregate failure missing:\n%s", output) } if data, err := os.ReadFile(filepath.Join(base, "dws", "SKILL.md")); err != nil || string(data) != "old mono\n" { t.Fatalf("mono changed after backup failure (data=%q, err=%v)", string(data), err) @@ -1246,6 +1256,64 @@ fi } } +func TestInstallerShellMultiCopyFailureReturnsFailure(t *testing.T) { + for _, scriptName := range []string{"install.sh", "install-skills.sh"} { + scriptName := scriptName + t.Run(scriptName, func(t *testing.T) { + t.Parallel() + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", scriptName)) + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(scriptPath) + if err != nil { + t.Fatal(err) + } + cut := strings.LastIndex(string(data), "\nmain\n") + if cut < 0 { + t.Fatalf("%s final main invocation not found", scriptName) + } + library := filepath.Join(t.TempDir(), scriptName+"-lib.sh") + mustWriteFile(t, library, data[:cut], 0o755) + + home := t.TempDir() + source := filepath.Join(t.TempDir(), "multi") + base := filepath.Join(home, ".agents", "skills") + mustWriteFile(t, filepath.Join(source, "dingtalk-test", "SKILL.md"), []byte("new skill\n"), 0o644) + + installCall := `install_multi_skills_to_homes "$DWS_TEST_SOURCE"` + if scriptName == "install-skills.sh" { + installCall = `install_multi_skills_to_root "$DWS_TEST_SOURCE" "$HOME"` + } + harness := `. "$DWS_TEST_LIBRARY" +cp() { return 1; } +if ` + installCall + `; then + exit 2 +fi +` + cmd := exec.Command("sh", "-c", harness) + cmd.Env = append(os.Environ(), + "HOME="+home, + "DWS_TEST_LIBRARY="+library, + "DWS_TEST_SOURCE="+source, + ) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%s copy-failure harness failed: %v\n%s", scriptName, err, output) + } + if strings.Contains(string(output), "✅ Skills") { + t.Fatalf("%s reported success after copy failure:\n%s", scriptName, output) + } + if !strings.Contains(string(output), "所有检测到的 Agent 目标均失败") { + t.Fatalf("%s did not report aggregate install failure:\n%s", scriptName, output) + } + if _, err := os.Stat(filepath.Join(base, "dingtalk-test", "SKILL.md")); !os.IsNotExist(err) { + t.Fatalf("%s left a completed Skill after copy failure: %v", scriptName, err) + } + }) + } +} + func TestInstallPowerShellCacheCopyFailurePreservesOldCache(t *testing.T) { pwsh, err := exec.LookPath("pwsh") if err != nil {