diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4d46a1ea..25dcd3f9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -458,15 +458,16 @@ jobs: PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -eu - test "$(git rev-parse HEAD^1)" = "$PR_BASE_SHA" || { - echo "focused test merge first parent does not match event base" >&2 - exit 1 - } test "$(git rev-parse HEAD^2)" = "$PR_HEAD_SHA" || { echo "focused test merge second parent does not match event head" >&2 exit 1 } - echo "TEST_BASE_REF=$PR_BASE_SHA" >> "$GITHUB_ENV" + merge_base_sha="$(git rev-parse HEAD^1)" + git merge-base --is-ancestor "$PR_BASE_SHA" "$merge_base_sha" || { + echo "focused test merge base is not a fast-forward of event base" >&2 + exit 1 + } + echo "TEST_BASE_REF=$merge_base_sha" >> "$GITHUB_ENV" echo "TEST_HEAD_REF=$(git rev-parse HEAD)" >> "$GITHUB_ENV" - name: Set up Go @@ -1255,26 +1256,27 @@ jobs: PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -eu - test "$(git rev-parse HEAD^1)" = "$PR_BASE_SHA" || { - echo "checked-out merge first parent does not match event base" >&2 - exit 1 - } test "$(git rev-parse HEAD^2)" = "$PR_HEAD_SHA" || { echo "checked-out merge second parent does not match event head" >&2 exit 1 } + merge_base_sha="$(git rev-parse HEAD^1)" + git merge-base --is-ancestor "$PR_BASE_SHA" "$merge_base_sha" || { + echo "checked-out merge base is not a fast-forward of event base" >&2 + exit 1 + } + echo "POLICY_BASE_SHA=$merge_base_sha" >> "$GITHUB_ENV" - name: Validate changed CHANGELOG content if: github.event_name == 'pull_request' env: CLASSIFIED_CHANGELOG_CHANGED: ${{ needs.lint.outputs.changelog_changed }} CHANGELOG_ONLY: ${{ needs.lint.outputs.changelog_only }} - PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | set -eu merge_changelog_changed=false if git diff --no-ext-diff --find-renames --name-status \ - "$PR_BASE_SHA" HEAD | + "$POLICY_BASE_SHA" HEAD | awk -F '\t' ' { for (field = 2; field <= NF; field++) { @@ -1299,7 +1301,7 @@ jobs: mode=--fast-path fi ./scripts/policy/check-changelog-pr.sh \ - "$mode" "$PR_BASE_SHA" HEAD + "$mode" "$POLICY_BASE_SHA" HEAD - name: Validate trusted main CHANGELOG-only push if: github.event_name == 'push' && needs.lint.outputs.changelog_only == 'true' @@ -1324,13 +1326,12 @@ jobs: - name: Validate scoped policy if: ${{ needs.lint.outputs.changelog_only != 'true' && (needs.lint.outputs.docs_only == 'true' || (needs.lint.outputs.full_suite != 'true' && needs.lint.outputs.interface_sensitive != 'true')) }} env: - PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} PUSH_BEFORE_SHA: ${{ github.event.before }} run: | set -eu base_ref="$PUSH_BEFORE_SHA" if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then - base_ref="$PR_BASE_SHA" + base_ref="$POLICY_BASE_SHA" fi git rev-parse --verify "${base_ref}^{commit}" >/dev/null git diff --check "$base_ref" HEAD diff --git a/README.md b/README.md index 1ae82893..b861cd07 100644 --- a/README.md +++ b/README.md @@ -432,7 +432,7 @@ DWS_SKILL_SOURCE=/path/to/skills dws skill setup --mode multi | `--source` | path | Local source directory (overrides bundled skills) | | `--yes` | — | Scripting-only: skip the confirmation prompt. Removals are still backed up to `~/.dws/skill-backups/` first | -> 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. +> 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, setup refuses to migrate directories unless `--yes` is explicitly supplied after reviewing `--dry-run` output. 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 5a906b19..fe2e92e9 100644 --- a/README_zh.md +++ b/README_zh.md @@ -426,7 +426,7 @@ DWS_SKILL_SOURCE=/path/to/skills dws skill setup --mode multi | `--source` | 路径 | 本地源目录(覆盖内置 skills) | | `--yes` | — | 仅供脚本使用:跳过确认提示。删除操作仍会先备份到 `~/.dws/skill-backups/` | -> setup 命令可能移除对面模式残留(装 multi 删 `dws/`,装 mono 删 `dingtalk-*`)以及不在 bundle 内的过期 skill。所有删除都会先列入确认预览,并备份到 `~/.dws/skill-backups/<时间戳>/`;备份失败的目录会保留原样、绝不删除。 +> setup 命令可能移除对面模式残留(装 multi 删 `dws/`,装 mono 删 `dingtalk-*`)以及不在 bundle 内的过期 skill。所有删除都会先列入确认预览,并备份到 `~/.dws/skill-backups/<时间戳>/`;备份失败的目录会保留原样、绝不删除。非交互环境默认拒绝迁移目录;请先用 `--dry-run` 核对,再显式传入 `--yes`。 环境变量:`DWS_SKILL_MODE=mono|multi`(`install.sh` / `install.ps1` 也认)、`DWS_SKILL_SOURCE=<路径>`。 diff --git a/build/npm/install.js b/build/npm/install.js index 2e411130..2af4db9b 100755 --- a/build/npm/install.js +++ b/build/npm/install.js @@ -169,6 +169,7 @@ function copyChildren(srcDir, destDir) { function installSkillsToHomes(skillRoot) { const homeDir = os.homedir(); let installed = 0; + let attempted = 0; AGENT_DIRS.forEach((agentDir, index) => { const baseDir = path.join(homeDir, agentDir); @@ -176,6 +177,7 @@ function installSkillsToHomes(skillRoot) { if (index > 0 && !fs.existsSync(parentGate)) { return; } + attempted += 1; // Mutual exclusion: back up + remove multi leftovers before laying down // mono. Directories only — a stray file named dingtalk-x.md must survive. // Non-interactive installs cannot confirm, so removals stay reversible via @@ -183,7 +185,10 @@ function installSkillsToHomes(skillRoot) { if (fs.existsSync(baseDir)) { for (const entry of fs.readdirSync(baseDir, { withFileTypes: true })) { if (entry.isDirectory() && (entry.name.startsWith("dingtalk-") || entry.name === "dws-shared")) { - backupAndRemoveSkillDir(homeDir, path.join(baseDir, entry.name)); + if (!backupAndRemoveSkillDir(homeDir, path.join(baseDir, entry.name))) { + console.warn(`⚠️ 跳过 ${baseDir}(multi 残留备份失败,未安装 mono)`); + return; + } } } } @@ -192,15 +197,18 @@ function installSkillsToHomes(skillRoot) { // Refreshing an existing skill: on backup failure keep the user's // copy and skip this target. console.warn(`⚠️ 跳过 ${destDir}(保留原目录)`); - installed += 1; return; } copyChildren(skillRoot, destDir); installed += 1; }); - if (installed === 0) { + if (attempted === 0) { copyChildren(skillRoot, path.join(homeDir, ".agents", "skills", "dws")); + installed += 1; + } + if (installed === 0) { + console.warn("⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败"); } } @@ -234,31 +242,43 @@ function installMultiSkillsToHomes(multiRoot) { } const skillSet = new Set(skills); let installed = 0; + let attempted = 0; const installToBase = (baseDir) => { fs.mkdirSync(baseDir, { recursive: true }); // Mutual exclusion: back up + remove the mono leftover, then stale multi // skills (dingtalk-* or dws-shared) not in the new bundle. - backupAndRemoveSkillDir(homeDir, path.join(baseDir, "dws")); + if (!backupAndRemoveSkillDir(homeDir, path.join(baseDir, "dws"))) { + console.warn(`⚠️ 跳过 ${baseDir}(mono 残留备份失败,未安装 multi)`); + return false; + } for (const entry of fs.readdirSync(baseDir, { withFileTypes: true })) { if ( entry.isDirectory() && (entry.name.startsWith("dingtalk-") || entry.name === "dws-shared") && !skillSet.has(entry.name) ) { - backupAndRemoveSkillDir(homeDir, path.join(baseDir, entry.name)); + if (!backupAndRemoveSkillDir(homeDir, path.join(baseDir, entry.name))) { + console.warn(`⚠️ 跳过 ${baseDir}(过期 Skill 备份失败,未安装 multi)`); + return false; + } + } + } + // Finish every potentially failing backup before copying the first new + // skill. This prevents a later backup failure from leaving a partial + // bundle in this Agent target. + for (const name of skills) { + const destDir = path.join(baseDir, name); + if (!backupAndRemoveSkillDir(homeDir, destDir)) { + console.warn(`⚠️ 跳过 ${baseDir}(${name} 备份失败,未安装 multi)`); + return false; } } for (const name of skills) { const destDir = path.join(baseDir, name); - if (!backupAndRemoveSkillDir(homeDir, destDir)) { - // Refreshing an existing skill: on backup failure keep the user's - // copy and skip this skill. - console.warn(`⚠️ 跳过 ${destDir}(保留原目录)`); - continue; - } copyChildren(path.join(multiRoot, name), destDir); } + return true; }; AGENT_DIRS.forEach((agentDir, index) => { @@ -267,12 +287,17 @@ function installMultiSkillsToHomes(multiRoot) { if (index > 0 && !fs.existsSync(parentGate)) { return; } - installToBase(baseDir); - installed += 1; + attempted += 1; + if (installToBase(baseDir)) { + installed += 1; + } }); + if (attempted === 0 && installToBase(path.join(homeDir, ".agents", "skills"))) { + installed += 1; + } if (installed === 0) { - installToBase(path.join(homeDir, ".agents", "skills")); + console.warn("⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败"); } } diff --git a/internal/app/skill_setup.go b/internal/app/skill_setup.go index 0b0e4efa..71593d16 100644 --- a/internal/app/skill_setup.go +++ b/internal/app/skill_setup.go @@ -587,7 +587,7 @@ func confirmSkillSetup(out io.Writer, mode, src string, dests []string, multiSki } if !skillSetupInteractive() { - return true, nil + return false, fmt.Errorf("非交互环境无法确认目录迁移;请先用 --dry-run 预览,确认后显式传入 --yes") } var confirm bool @@ -646,35 +646,37 @@ func mutualExclusionVictims(dest, mode string) ([]string, error) { return nil, nil } -// cleanupMutualExclusion best-effort backs up + removes the opposite-mode +// cleanupMutualExclusion backs up + removes the opposite-mode // leftovers. Removals are reversible: each victim is moved to // ~/.dws/skill-backups// first (skillSetupBackupAndRemove), and a // victim whose backup fails is left in place with a warning instead of being -// destroyed. Failures — including a failed victim scan — emit a warning to -// errOut but never abort the install. -func cleanupMutualExclusion(dest, mode string, out, errOut io.Writer) { +// destroyed. A failure is returned so the caller skips the complete Agent +// target and never installs both layouts together. +func cleanupMutualExclusion(dest, mode string, out, errOut io.Writer) error { victims, scanErr := mutualExclusionVictims(dest, mode) if scanErr != nil { - fmt.Fprintf(errOut, " ⚠️ 互斥清理扫描失败(继续安装) %s: %v\n", dest, scanErr) + fmt.Fprintf(errOut, " ⚠️ 互斥清理扫描失败(跳过整个 Agent 目标) %s: %v\n", dest, scanErr) + return scanErr } if len(victims) == 0 { - return + return nil } home, homeErr := skillSetupUserHomeDir() if homeErr != nil { for _, victim := range victims { fmt.Fprintf(errOut, " ⚠️ 无法解析 HOME,跳过删除(保留) %s: %v\n", victim, homeErr) } - return + return homeErr } for _, victim := range victims { backup, err := skillSetupBackupAndRemove(home, victim) if err != nil { - fmt.Fprintf(errOut, " ⚠️ 互斥清理失败(保留原目录,继续安装) %s: %v\n", victim, err) - continue + fmt.Fprintf(errOut, " ⚠️ 互斥清理失败(保留原目录,跳过整个 Agent 目标) %s: %v\n", victim, err) + return err } fmt.Fprintf(out, " × 已备份并清理对面模式残留 %s → %s\n", victim, backup) } + return nil } func installSkillToHomes(src string, dests []string, out, errOut io.Writer) (installed, skipped int, err error) { @@ -682,7 +684,11 @@ func installSkillToHomes(src string, dests []string, out, errOut io.Writer) (ins sort.Strings(dests) for _, dest := range dests { // 先做互斥清理:装 mono 前先把同级 dingtalk-* 子目录全部备份移除 - cleanupMutualExclusion(dest, skillSetupModeMono, out, errOut) + if cleanupErr := cleanupMutualExclusion(dest, skillSetupModeMono, out, errOut); cleanupErr != nil { + fmt.Fprintf(errOut, " ✗ 互斥清理失败,跳过整个 Agent 目标 %s: %v\n", dest, cleanupErr) + skipped++ + continue + } // 刷新同名 dest 前先备份:已存在的 dws/ 可能带有用户修改, // 直接 RemoveAll 会不可恢复。备份失败(含 HOME 解析失败)则保留 @@ -727,7 +733,11 @@ func installMultiSkillToHomes(src string, skillNames []string, dests []string, o sort.Strings(dests) for _, dest := range dests { // 互斥清理:装 multi 前先把 dest/dws/ 整个备份移除(mono 残留) - cleanupMutualExclusion(dest, skillSetupModeMulti, out, errOut) + if cleanupErr := cleanupMutualExclusion(dest, skillSetupModeMulti, out, errOut); cleanupErr != nil { + fmt.Fprintf(errOut, " ✗ 互斥清理失败,跳过整个 Agent 目标 %s: %v\n", dest, cleanupErr) + skipped += len(skillNames) + continue + } if err := skillSetupMkdirAll(dest, 0o755); err != nil { fmt.Fprintf(errOut, " ✗ Agent 目录创建失败 %s: %v\n", dest, err) @@ -736,24 +746,37 @@ func installMultiSkillToHomes(src string, skillNames []string, dests []string, o } if !filtered { - removeStaleMultiSkills(dest, skillNames, out, errOut) + if cleanupErr := removeStaleMultiSkills(dest, skillNames, out, errOut); cleanupErr != nil { + fmt.Fprintf(errOut, " ✗ 过期 Skill 备份失败,跳过整个 Agent 目标 %s: %v\n", dest, cleanupErr) + skipped += len(skillNames) + continue + } + } + + // Finish every destination backup before copying the first new Skill. + // Otherwise a later backup failure could leave a partial bundle. + backupFailed := false + for _, name := range skillNames { + subDest := filepath.Join(dest, name) + if homeErr != nil { + fmt.Fprintf(errOut, " ✗ 无法解析 HOME,跳过整个 Agent 目标 %s: %v\n", dest, homeErr) + backupFailed = true + break + } + if _, backupErr := skillSetupBackupAndRemove(home, subDest); backupErr != nil { + fmt.Fprintf(errOut, " ✗ 备份旧 Skill 失败,跳过整个 Agent 目标 %s: %v\n", dest, backupErr) + backupFailed = true + break + } + } + if backupFailed { + skipped += len(skillNames) + continue } for _, name := range skillNames { subSrc := filepath.Join(src, name) subDest := filepath.Join(dest, name) - // 刷新同名子 skill 前先备份(与 mono 路径一致),备份失败保留 - // 原目录并跳过。 - if homeErr != nil { - fmt.Fprintf(errOut, " ✗ 无法解析 HOME,跳过刷新(保留原目录) %s: %v\n", subDest, homeErr) - skipped++ - continue - } - if _, backupErr := skillSetupBackupAndRemove(home, subDest); backupErr != nil { - fmt.Fprintf(errOut, " ✗ 备份旧 skill 失败(保留原目录) %s: %v\n", subDest, backupErr) - skipped++ - continue - } if err := skillSetupCopyDir(subSrc, subDest); err != nil { fmt.Fprintf(errOut, " ✗ 拷贝失败 %s: %v\n", subDest, err) skipped++ @@ -798,15 +821,16 @@ func staleMultiSkillVictims(dest string, keep []string) []string { // directories under dest that are not part of the current bundle. Removal is // reversible: each stale directory is moved to ~/.dws/skill-backups// // first, and a backup failure keeps the directory in place with a warning. -// Best-effort: scan/removal failures warn on errOut and never abort the -// install. -func removeStaleMultiSkills(dest string, keep []string, out, errOut io.Writer) { +// A scan or backup failure is returned so callers do not write a new bundle +// into a partially reconciled Agent target. +func removeStaleMultiSkills(dest string, keep []string, out, errOut io.Writer) error { entries, err := skillSetupReadDir(dest) if err != nil { - if !errors.Is(err, os.ErrNotExist) { - fmt.Fprintf(errOut, " ⚠️ 过期 skill 扫描失败(继续安装) %s: %v\n", dest, err) + if errors.Is(err, os.ErrNotExist) { + return nil } - return + fmt.Fprintf(errOut, " ⚠️ 过期 skill 扫描失败(跳过整个 Agent 目标) %s: %v\n", dest, err) + return err } keepSet := make(map[string]bool, len(keep)) for _, n := range keep { @@ -823,23 +847,24 @@ func removeStaleMultiSkills(dest string, keep []string, out, errOut io.Writer) { stales = append(stales, filepath.Join(dest, e.Name())) } if len(stales) == 0 { - return + return nil } home, homeErr := skillSetupUserHomeDir() if homeErr != nil { for _, stale := range stales { fmt.Fprintf(errOut, " ⚠️ 无法解析 HOME,跳过删除(保留) %s: %v\n", stale, homeErr) } - return + return homeErr } for _, stale := range stales { backup, err := skillSetupBackupAndRemove(home, stale) if err != nil { - fmt.Fprintf(errOut, " ⚠️ 过期 skill 清理失败(保留原目录,继续安装) %s: %v\n", stale, err) - continue + fmt.Fprintf(errOut, " ⚠️ 过期 skill 清理失败(保留原目录,跳过整个 Agent 目标) %s: %v\n", stale, err) + return err } fmt.Fprintf(out, " × 已备份并清理过期 skill %s → %s\n", stale, backup) } + return nil } func copyDir(src, dst string) error { diff --git a/internal/app/skill_setup_cleanup_coverage_test.go b/internal/app/skill_setup_cleanup_coverage_test.go index e2048823..e78802af 100644 --- a/internal/app/skill_setup_cleanup_coverage_test.go +++ b/internal/app/skill_setup_cleanup_coverage_test.go @@ -32,8 +32,8 @@ func TestCrossPlatformCoverageSkillSetupConfirmPreviewsStaleSkills(t *testing.T) var out bytes.Buffer ok, err := confirmSkillSetup(&out, skillSetupModeMulti, "src", []string{dest}, []string{"dingtalk-chat"}, false) - if err != nil || !ok { - t.Fatalf("confirmSkillSetup = (%v, %v), want (true, nil)", ok, err) + if err == nil || ok || !strings.Contains(err.Error(), "--yes") { + t.Fatalf("confirmSkillSetup = (%v, %v), want non-interactive confirmation error", ok, err) } if !strings.Contains(out.String(), "将备份并移除过期 skill") { t.Fatalf("full install preview must list stale skills, got %q", out.String()) @@ -44,8 +44,8 @@ func TestCrossPlatformCoverageSkillSetupConfirmPreviewsStaleSkills(t *testing.T) out.Reset() ok, err = confirmSkillSetup(&out, skillSetupModeMulti, "src", []string{dest}, []string{"dingtalk-chat"}, true) - if err != nil || !ok { - t.Fatalf("filtered confirmSkillSetup = (%v, %v), want (true, nil)", ok, err) + if err == nil || ok { + t.Fatalf("filtered confirmSkillSetup = (%v, %v), want non-interactive confirmation error", ok, err) } if strings.Contains(out.String(), "将备份并移除过期 skill") { t.Fatalf("filtered install must stay additive in the preview, got %q", out.String()) @@ -77,6 +77,75 @@ func TestCrossPlatformCoverageSkillSetupCleanupHomeFailure(t *testing.T) { } } +func TestCrossPlatformCoverageSkillSetupBackupFailureSkipsWholeTarget(t *testing.T) { + home := t.TempDir() + testseam.Swap(t, &skillSetupUserHomeDir, func() (string, error) { return home, nil }) + copyCalls := 0 + testseam.Swap(t, &skillSetupCopyDir, func(string, string) error { + copyCalls++ + return nil + }) + failure := errors.New("backup boom") + testseam.Swap(t, &skillSetupBackupAndRemove, func(_ string, dir string) (string, error) { + if filepath.Base(dir) == "dws" { + return "", failure + } + return "", nil + }) + + dest := filepath.Join(home, ".agents", "skills") + if err := os.MkdirAll(filepath.Join(dest, "dws"), 0o755); err != nil { + t.Fatal(err) + } + src := writeMultiSkillSource(t, []string{"dingtalk-a", "dingtalk-shared"}) + var out, errOut bytes.Buffer + installed, skipped, err := installMultiSkillToHomes(src, []string{"dingtalk-a", "dingtalk-shared"}, []string{dest}, &out, &errOut, false) + if err != nil || installed != 0 || skipped != 2 { + t.Fatalf("install = (%d, %d, %v), want (0, 2, nil)", installed, skipped, err) + } + if copyCalls != 0 { + t.Fatalf("backup failure copied %d new Skills", copyCalls) + } + if !strings.Contains(errOut.String(), "跳过整个 Agent 目标") { + t.Fatalf("missing whole-target warning: %q", errOut.String()) + } +} + +func TestCrossPlatformCoverageSkillSetupStaleBackupFailureSkipsWholeTarget(t *testing.T) { + home := t.TempDir() + testseam.Swap(t, &skillSetupUserHomeDir, func() (string, error) { return home, nil }) + copyCalls := 0 + testseam.Swap(t, &skillSetupCopyDir, func(string, string) error { + copyCalls++ + return nil + }) + failure := errors.New("stale backup boom") + testseam.Swap(t, &skillSetupBackupAndRemove, func(_ string, dir string) (string, error) { + if filepath.Base(dir) == "dingtalk-stale" { + return "", failure + } + return "", nil + }) + + dest := filepath.Join(home, ".agents", "skills") + stale := filepath.Join(dest, "dingtalk-stale") + if err := os.MkdirAll(stale, 0o755); err != nil { + t.Fatal(err) + } + src := writeMultiSkillSource(t, []string{"dingtalk-a", "dingtalk-shared"}) + var out, errOut bytes.Buffer + installed, skipped, err := installMultiSkillToHomes(src, []string{"dingtalk-a", "dingtalk-shared"}, []string{dest}, &out, &errOut, false) + if err != nil || installed != 0 || skipped != 2 { + t.Fatalf("install = (%d, %d, %v), want (0, 2, nil)", installed, skipped, err) + } + if copyCalls != 0 { + t.Fatalf("stale backup failure copied %d new Skills", copyCalls) + } + if !strings.Contains(errOut.String(), "过期 Skill 备份失败,跳过整个 Agent 目标") { + t.Fatalf("missing stale whole-target warning: %q", errOut.String()) + } +} + // TestCrossPlatformCoverageSkillSetupInstallHomeFailureSkips verifies both // install paths skip (never destroy) every target when $HOME cannot be // resolved for the pre-refresh backup. @@ -117,7 +186,7 @@ func TestCrossPlatformCoverageSkillSetupInstallHomeFailureSkips(t *testing.T) { if err != nil || installed != 0 || skipped != 1 { t.Fatalf("multi install = (%d, %d, %v), want (0, 1, nil)", installed, skipped, err) } - if !strings.Contains(errOut.String(), "无法解析 HOME,跳过刷新") { + if !strings.Contains(errOut.String(), "无法解析 HOME,跳过整个 Agent 目标") { t.Fatalf("expected multi HOME skip warning, got %q", errOut.String()) } if _, err := os.Stat(filepath.Join(multiDest, "dingtalk-a")); err != nil { diff --git a/internal/app/skill_setup_test.go b/internal/app/skill_setup_test.go index ba099b7c..08343b68 100644 --- a/internal/app/skill_setup_test.go +++ b/internal/app/skill_setup_test.go @@ -117,6 +117,72 @@ func TestCrossPlatformCoverageSkillSetupDeclinedConfirmationNeverRemoves(t *test } } +// TestCrossPlatformCoverageSkillSetupNonInteractiveRequiresYes pins the real +// non-TTY safety boundary: an explicit mode alone is not consent to move +// directories. The same command with --yes performs the previewed backup and +// installation. +func TestCrossPlatformCoverageSkillSetupNonInteractiveRequiresYes(t *testing.T) { + home := t.TempDir() + setTestHome(t, home) + oldInteractive := skillSetupInteractive + skillSetupInteractive = func() bool { return false } + t.Cleanup(func() { skillSetupInteractive = oldInteractive }) + + multiSrc := writeMultiSkillSource(t, []string{"dingtalk-aitable"}) + agentHome := filepath.Join(home, ".claude", "skills") + mono := filepath.Join(agentHome, "dws") + if err := os.MkdirAll(mono, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(mono, "SKILL.md"), []byte("keep-me"), 0o644); err != nil { + t.Fatal(err) + } + + run := func(extra ...string) error { + cmd := newSkillSetupCommand() + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + args := []string{"--mode", "multi", "--target", "claude", "--source", multiSrc} + cmd.SetArgs(append(args, extra...)) + return cmd.Execute() + } + + if err := run(); err == nil || !strings.Contains(err.Error(), "--yes") { + t.Fatalf("non-interactive setup error = %v, want explicit --yes requirement", err) + } + if data, err := os.ReadFile(filepath.Join(mono, "SKILL.md")); err != nil || string(data) != "keep-me" { + t.Fatalf("unconfirmed setup changed mono (data=%q, err=%v)", string(data), err) + } + if _, err := os.Stat(filepath.Join(agentHome, "dingtalk-aitable")); !os.IsNotExist(err) { + t.Fatalf("unconfirmed setup installed multi, stat err=%v", err) + } + if _, err := os.Stat(filepath.Join(home, ".dws", "skill-backups")); !os.IsNotExist(err) { + t.Fatalf("unconfirmed setup created backup state, stat err=%v", err) + } + + if err := run("--yes"); err != nil { + t.Fatalf("explicitly confirmed setup failed: %v", err) + } + if _, err := os.Stat(mono); !os.IsNotExist(err) { + t.Fatalf("confirmed setup kept mono, stat err=%v", err) + } + if _, err := os.Stat(filepath.Join(agentHome, "dingtalk-aitable", "SKILL.md")); err != nil { + t.Fatalf("confirmed setup did not install multi: %v", err) + } + backupFound := false + _ = filepath.Walk(filepath.Join(home, ".dws", "skill-backups"), func(path string, info os.FileInfo, walkErr error) error { + if walkErr == nil && info != nil && !info.IsDir() && info.Name() == "SKILL.md" { + if data, readErr := os.ReadFile(path); readErr == nil && string(data) == "keep-me" { + backupFound = true + } + } + return nil + }) + if !backupFound { + t.Fatal("confirmed setup did not back up mono") + } +} + func TestResolveSkillSetupModeFlagDirect(t *testing.T) { got, err := resolveSkillSetupMode("mono", true, &bytes.Buffer{}) if err != nil || got != skillSetupModeMono { diff --git a/internal/upgrade/paths.go b/internal/upgrade/paths.go index 045470ab..f75cf2bd 100644 --- a/internal/upgrade/paths.go +++ b/internal/upgrade/paths.go @@ -413,6 +413,12 @@ func upgradeMultiSkillLocations(homeDir, multiRoot string, skills []string) (*Sk failed = true break } + } + if failed { + continue + } + for _, name := range skills { + subDest := filepath.Join(destBase, name) if err := upgradeCopyDir(filepath.Join(multiRoot, name), subDest); err != nil { result.Results = append(result.Results, SkillDirResult{Dir: destBase, Status: SkillDirFailed, Err: err}) failed = true @@ -437,6 +443,9 @@ func upgradeMultiSkillLocations(homeDir, multiRoot string, skills []string) (*Sk if _, err := backupAndRemoveSkillDir(homeDir, subDest); err != nil { return result, fmt.Errorf("所有技能目录安装失败,回退到主目录备份残留也失败: %w", err) } + } + for _, name := range skills { + subDest := filepath.Join(destBase, name) if err := upgradeCopyDir(filepath.Join(multiRoot, name), subDest); err != nil { return result, fmt.Errorf("所有技能目录安装失败,回退到主目录也失败: %w", err) } diff --git a/scripts/install-skills.sh b/scripts/install-skills.sh index 29e4d079..f524a370 100755 --- a/scripts/install-skills.sh +++ b/scripts/install-skills.sh @@ -156,7 +156,7 @@ _copy_skill_summary() { if [ -d "$_dest" ]; then backup_and_remove_skill_dir "$_dest" || { printf ' ⚠️ 跳过 %s(保留原目录)\n' "$_dest" - return 0 + return 1 } fi @@ -176,7 +176,7 @@ _copy_skill() { if [ -d "$_dest" ]; then backup_and_remove_skill_dir "$_dest" || { printf ' ⚠️ 跳过 %s(保留原目录)\n' "$_dest" - return 0 + return 1 } fi @@ -218,6 +218,7 @@ install_multi_skills_to_root() { multi_src="$1" root="$2" installed=0 + attempted=0 idx=0 for agent_dir in \ ".agents/skills" \ @@ -243,12 +244,19 @@ install_multi_skills_to_root() { idx=$((idx + 1)) continue fi - _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir" - installed=$((installed + 1)) + attempted=$((attempted + 1)) + if _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir"; then + installed=$((installed + 1)) + else + printf ' ⚠️ 跳过 %s(备份失败,未安装 multi)\n' "$base_dir" + fi idx=$((idx + 1)) done + if [ "$attempted" -eq 0 ] && _install_multi_to_base "$multi_src" "$root/.agents/skills" "$root" ".agents/skills"; then + installed=$((installed + 1)) + fi if [ "$installed" -eq 0 ]; then - _install_multi_to_base "$multi_src" "$root/.agents/skills" "$root" ".agents/skills" + printf ' ⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败\n' fi } @@ -258,10 +266,10 @@ _install_multi_to_base() { _root="$3" _agent_dir="$4" - mkdir -p "$_base" + mkdir -p "$_base" || return 1 # Mutual exclusion: back up + remove the mono leftover. - backup_and_remove_skill_dir "$_base/$SKILL_NAME" || true + backup_and_remove_skill_dir "$_base/$SKILL_NAME" || return 1 # Back up + remove stale multi skills (dingtalk-* or dws-shared) not in the # new bundle. @@ -269,24 +277,28 @@ _install_multi_to_base() { [ -d "$existing" ] || continue _name="$(basename "$existing")" if [ ! -f "$_msrc/$_name/SKILL.md" ]; then - backup_and_remove_skill_dir "$existing" || true + backup_and_remove_skill_dir "$existing" || return 1 fi done if [ -d "$_base/dws-shared" ] && [ ! -f "$_msrc/dws-shared/SKILL.md" ]; then - backup_and_remove_skill_dir "$_base/dws-shared" || true + backup_and_remove_skill_dir "$_base/dws-shared" || return 1 fi + # Complete all backups before copying the first new skill so a later + # failure cannot leave a partial multi bundle in this Agent target. + for skill_dir in "$_msrc"/*/; do + [ -f "${skill_dir}SKILL.md" ] || continue + _name="$(basename "$skill_dir")" + _dest="$_base/$_name" + backup_and_remove_skill_dir "$_dest" || return 1 + done + _count=0 for skill_dir in "$_msrc"/*/; do [ -f "${skill_dir}SKILL.md" ] || continue _name="$(basename "$skill_dir")" _dest="$_base/$_name" - if [ -d "$_dest" ]; then - # Refreshing an existing skill: back it up first; on backup failure - # keep the user's copy and skip this skill. - backup_and_remove_skill_dir "$_dest" || continue - fi - mkdir -p "$_dest" + mkdir -p "$_dest" || return 1 cp -R "${skill_dir}." "$_dest/" 2>/dev/null || cp -r "${skill_dir}." "$_dest/" _count=$((_count + 1)) done @@ -304,6 +316,7 @@ install_skills_to_root() { skill_src="$1" root="$2" installed=0 + attempted=0 idx=0 for agent_dir in \ ".agents/skills" \ @@ -329,14 +342,29 @@ install_skills_to_root() { idx=$((idx + 1)) continue fi + attempted=$((attempted + 1)) # Mutual exclusion: back up + remove multi leftovers before laying down # mono. Non-interactive installs cannot confirm, so removals stay # reversible via ~/.dws/skill-backups/ (backup failure keeps the dir). - backup_and_remove_skill_dir "$base_dir/dws-shared" || true + cleanup_ok=1 + backup_and_remove_skill_dir "$base_dir/dws-shared" || cleanup_ok=0 + if [ "$cleanup_ok" -ne 1 ]; then + printf ' ⚠️ 跳过 %s(multi 残留备份失败,未安装 mono)\n' "$base_dir" + idx=$((idx + 1)) + continue + fi for existing in "$base_dir"/dingtalk-*/; do [ -d "$existing" ] || continue - backup_and_remove_skill_dir "$existing" || true + backup_and_remove_skill_dir "$existing" || { + cleanup_ok=0 + break + } done + if [ "$cleanup_ok" -ne 1 ]; then + printf ' ⚠️ 跳过 %s(multi 残留备份失败,未安装 mono)\n' "$base_dir" + idx=$((idx + 1)) + continue + fi dest="$base_dir/$SKILL_NAME" if [ "$root" = "$HOME" ]; then label="~/$agent_dir/$SKILL_NAME" @@ -344,20 +372,26 @@ install_skills_to_root() { label="$root/$agent_dir/$SKILL_NAME" fi if [ "$installed" -eq 0 ]; then - _copy_skill "$skill_src" "$dest" "$label" - else - _copy_skill_summary "$skill_src" "$dest" "$label" + if _copy_skill "$skill_src" "$dest" "$label"; then + installed=$((installed + 1)) + fi + elif _copy_skill_summary "$skill_src" "$dest" "$label"; then + installed=$((installed + 1)) fi - installed=$((installed + 1)) idx=$((idx + 1)) done - if [ "$installed" -eq 0 ]; then + if [ "$attempted" -eq 0 ]; then if [ "$root" = "$HOME" ]; then flabel="~/.agents/skills/$SKILL_NAME" else flabel="$root/.agents/skills/$SKILL_NAME" fi - _copy_skill "$skill_src" "$root/.agents/skills/$SKILL_NAME" "$flabel" + if _copy_skill "$skill_src" "$root/.agents/skills/$SKILL_NAME" "$flabel"; then + installed=$((installed + 1)) + fi + fi + if [ "$installed" -eq 0 ]; then + printf ' ⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败\n' fi } diff --git a/scripts/install.ps1 b/scripts/install.ps1 index c1021681..79064121 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -269,7 +269,7 @@ function Copy-SkillToDir { # the user's copy and skip this target. if (!(Backup-SkillDir -Dir $Dest)) { Write-Say "⚠️ 跳过 $Dest(保留原目录)" - return + return $false } $fileCount = Copy-DirRecursive -Source $SkillSrc -Destination $Dest @@ -284,6 +284,7 @@ function Copy-SkillToDir { Write-Say " 📄 $($_.Name)" } } + return $true } function Copy-SkillToDirSummary { @@ -291,11 +292,12 @@ function Copy-SkillToDirSummary { if (!(Backup-SkillDir -Dir $Dest)) { Write-Say "⚠️ 跳过 $Dest(保留原目录)" - return + return $false } $fileCount = Copy-DirRecursive -Source $SkillSrc -Destination $Dest Write-Say "✅ Skills → $Label ($fileCount files)" + return $true } function Resolve-SourceRoot { @@ -515,6 +517,7 @@ function Install-SkillsToHomes { ) $installed = 0 + $attempted = 0 for ($i = 0; $i -lt $AgentDirs.Count; $i++) { $agentDir = $AgentDirs[$i] $baseDir = Join-Path $Root $agentDir @@ -522,13 +525,24 @@ function Install-SkillsToHomes { if ($i -gt 0 -and !(Test-Path $parentGate)) { continue } + $attempted++ # Mutual exclusion: back up + remove multi leftovers before laying # down mono. Non-interactive installs cannot confirm, so removals # stay reversible via ~\.dws\skill-backups\ (backup failure keeps # the dir). - Backup-SkillDir -Dir (Join-Path $baseDir "dws-shared") | Out-Null - Get-ChildItem -Path $baseDir -Directory -Filter "dingtalk-*" -ErrorAction SilentlyContinue | - ForEach-Object { Backup-SkillDir -Dir $_.FullName | Out-Null } + $cleanupOK = Backup-SkillDir -Dir (Join-Path $baseDir "dws-shared") + if ($cleanupOK) { + foreach ($existing in Get-ChildItem -Path $baseDir -Directory -Filter "dingtalk-*" -ErrorAction SilentlyContinue) { + if (!(Backup-SkillDir -Dir $existing.FullName)) { + $cleanupOK = $false + break + } + } + } + if (!$cleanupOK) { + Write-Say "⚠️ 跳过 $baseDir(multi 残留备份失败,未安装 mono)" + continue + } $dest = Join-Path $baseDir $SkillName if ($Root -eq $HOME) { $label = "~\$agentDir\$SkillName" @@ -536,20 +550,27 @@ function Install-SkillsToHomes { $label = Join-Path $Root (Join-Path $agentDir $SkillName) } if ($installed -eq 0) { - Copy-SkillToDir -SkillSrc $SkillSrc -Dest $dest -Label $label + $copied = Copy-SkillToDir -SkillSrc $SkillSrc -Dest $dest -Label $label } else { - Copy-SkillToDirSummary -SkillSrc $SkillSrc -Dest $dest -Label $label + $copied = Copy-SkillToDirSummary -SkillSrc $SkillSrc -Dest $dest -Label $label + } + if ($copied) { + $installed++ } - $installed++ } - if ($installed -eq 0) { + if ($attempted -eq 0) { $fallback = Join-Path (Join-Path $Root ".agents\skills") $SkillName if ($Root -eq $HOME) { $flabel = "~\.agents\skills\$SkillName" } else { $flabel = Join-Path $Root (Join-Path ".agents\skills" $SkillName) } - Copy-SkillToDir -SkillSrc $SkillSrc -Dest $fallback -Label $flabel + if (Copy-SkillToDir -SkillSrc $SkillSrc -Dest $fallback -Label $flabel) { + $installed++ + } + } + if ($installed -eq 0) { + Write-Say "⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败" } } @@ -577,6 +598,7 @@ function Install-MultiSkillsToHomes { ) $installed = 0 + $attempted = 0 for ($i = 0; $i -lt $AgentDirs.Count; $i++) { $agentDir = $AgentDirs[$i] $baseDir = Join-Path $Root $agentDir @@ -584,11 +606,18 @@ function Install-MultiSkillsToHomes { if ($i -gt 0 -and !(Test-Path $parentGate)) { continue } - Install-MultiToBase -MultiSrc $MultiSrc -BaseDir $baseDir -Root $Root -AgentDir $agentDir + $attempted++ + if (Install-MultiToBase -MultiSrc $MultiSrc -BaseDir $baseDir -Root $Root -AgentDir $agentDir) { + $installed++ + } else { + Write-Say "⚠️ 跳过 $baseDir(备份失败,未安装 multi)" + } + } + if ($attempted -eq 0 -and (Install-MultiToBase -MultiSrc $MultiSrc -BaseDir (Join-Path $Root ".agents\skills") -Root $Root -AgentDir ".agents\skills")) { $installed++ } if ($installed -eq 0) { - Install-MultiToBase -MultiSrc $MultiSrc -BaseDir (Join-Path $Root ".agents\skills") -Root $Root -AgentDir ".agents\skills" + Write-Say "⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败" } } @@ -605,30 +634,41 @@ function Install-MultiToBase { } # Mutual exclusion: back up + remove the mono leftover. - Backup-SkillDir -Dir (Join-Path $BaseDir $SkillName) | Out-Null + if (!(Backup-SkillDir -Dir (Join-Path $BaseDir $SkillName))) { + return $false + } # Back up + remove stale multi skills (dingtalk-* or dws-shared) not in # the new bundle. - Get-ChildItem -Path $BaseDir -Directory -Filter "dingtalk-*" -ErrorAction SilentlyContinue | - Where-Object { !(Test-Path (Join-Path (Join-Path $MultiSrc $_.Name) "SKILL.md")) } | - ForEach-Object { Backup-SkillDir -Dir $_.FullName | Out-Null } + foreach ($existing in Get-ChildItem -Path $BaseDir -Directory -Filter "dingtalk-*" -ErrorAction SilentlyContinue) { + if (!(Test-Path (Join-Path (Join-Path $MultiSrc $existing.Name) "SKILL.md")) -and + !(Backup-SkillDir -Dir $existing.FullName)) { + return $false + } + } if (!(Test-Path (Join-Path (Join-Path $MultiSrc "dws-shared") "SKILL.md"))) { - Backup-SkillDir -Dir (Join-Path $BaseDir "dws-shared") | Out-Null + if (!(Backup-SkillDir -Dir (Join-Path $BaseDir "dws-shared"))) { + return $false + } + } + + $skillDirs = @(Get-ChildItem -Path $MultiSrc -Directory | Where-Object { + Test-Path (Join-Path $_.FullName "SKILL.md") + }) + # Complete all backups before copying the first new skill, preventing a + # later failure from leaving a partial multi bundle in this Agent target. + foreach ($skillDir in $skillDirs) { + if (!(Backup-SkillDir -Dir (Join-Path $BaseDir $skillDir.Name))) { + return $false + } } $count = 0 - Get-ChildItem -Path $MultiSrc -Directory | ForEach-Object { - $name = $_.Name - if (!(Test-Path (Join-Path $_.FullName "SKILL.md"))) { return } + foreach ($skillDir in $skillDirs) { + $name = $skillDir.Name $dest = Join-Path $BaseDir $name - if (!(Backup-SkillDir -Dir $dest)) { - # Refreshing an existing skill: on backup failure keep the - # user's copy and skip this skill. - Write-Say "⚠️ 跳过 $dest(保留原目录)" - return - } New-Item -ItemType Directory -Path $dest -Force | Out-Null - Copy-DirRecursive -Source $_.FullName -Destination $dest | Out-Null + Copy-DirRecursive -Source $skillDir.FullName -Destination $dest | Out-Null $count++ } @@ -638,6 +678,7 @@ function Install-MultiToBase { $label = Join-Path $Root $AgentDir } Write-Say "✅ Skills → $label ($count product skills)" + return $true } # ── Install Binary from Source ─────────────────────────────────────────────── diff --git a/scripts/install.sh b/scripts/install.sh index 61cabd5d..adfe0065 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -401,6 +401,7 @@ install_skills_to_homes() { skill_src="$1" root="${HOME}" installed=0 + attempted=0 idx=0 for agent_dir in \ ".agents/skills" \ @@ -426,14 +427,29 @@ install_skills_to_homes() { idx=$((idx + 1)) continue fi + attempted=$((attempted + 1)) # Mutual exclusion: back up + remove multi leftovers before laying down # mono. Non-interactive installs cannot confirm, so removals stay # reversible via ~/.dws/skill-backups/ (backup failure keeps the dir). - backup_and_remove_skill_dir "$base_dir/dws-shared" || true + cleanup_ok=1 + backup_and_remove_skill_dir "$base_dir/dws-shared" || cleanup_ok=0 + if [ "$cleanup_ok" -ne 1 ]; then + say " ⚠️ 跳过 ${base_dir}(multi 残留备份失败,未安装 mono)" + idx=$((idx + 1)) + continue + fi for existing in "$base_dir"/dingtalk-*/; do [ -d "$existing" ] || continue - backup_and_remove_skill_dir "$existing" || true + backup_and_remove_skill_dir "$existing" || { + cleanup_ok=0 + break + } done + if [ "$cleanup_ok" -ne 1 ]; then + say " ⚠️ 跳过 ${base_dir}(multi 残留备份失败,未安装 mono)" + idx=$((idx + 1)) + continue + fi dest="$base_dir/$SKILL_NAME" case "$root" in "$HOME") @@ -444,14 +460,15 @@ install_skills_to_homes() { ;; esac if [ "$installed" -eq 0 ]; then - _copy_skill "$skill_src" "$dest" "$label" - else - _copy_skill_summary "$skill_src" "$dest" "$label" + if _copy_skill "$skill_src" "$dest" "$label"; then + installed=$((installed + 1)) + fi + elif _copy_skill_summary "$skill_src" "$dest" "$label"; then + installed=$((installed + 1)) fi - installed=$((installed + 1)) idx=$((idx + 1)) done - if [ "$installed" -eq 0 ]; then + if [ "$attempted" -eq 0 ]; then case "$root" in "$HOME") flabel="~/.agents/skills/$SKILL_NAME" @@ -460,7 +477,12 @@ install_skills_to_homes() { flabel="$root/.agents/skills/$SKILL_NAME" ;; esac - _copy_skill "$skill_src" "$root/.agents/skills/$SKILL_NAME" "$flabel" + if _copy_skill "$skill_src" "$root/.agents/skills/$SKILL_NAME" "$flabel"; then + installed=$((installed + 1)) + fi + fi + if [ "$installed" -eq 0 ]; then + say " ⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败" fi } @@ -488,6 +510,7 @@ install_multi_skills_to_homes() { multi_src="$1" root="${HOME}" installed=0 + attempted=0 idx=0 for agent_dir in \ ".agents/skills" \ @@ -513,12 +536,19 @@ install_multi_skills_to_homes() { idx=$((idx + 1)) continue fi - _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir" - installed=$((installed + 1)) + attempted=$((attempted + 1)) + if _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir"; then + installed=$((installed + 1)) + else + say " ⚠️ 跳过 ${base_dir}(备份失败,未安装 multi)" + fi idx=$((idx + 1)) done + if [ "$attempted" -eq 0 ] && _install_multi_to_base "$multi_src" "$root/.agents/skills" "$root" ".agents/skills"; then + installed=$((installed + 1)) + fi if [ "$installed" -eq 0 ]; then - _install_multi_to_base "$multi_src" "$root/.agents/skills" "$root" ".agents/skills" + say " ⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败" fi } @@ -528,10 +558,10 @@ _install_multi_to_base() { _root="$3" _agent_dir="$4" - mkdir -p "$_base" + mkdir -p "$_base" || return 1 # Mutual exclusion: back up + remove the mono leftover. - backup_and_remove_skill_dir "$_base/$SKILL_NAME" || true + backup_and_remove_skill_dir "$_base/$SKILL_NAME" || return 1 # Back up + remove stale multi skills (dingtalk-* or dws-shared) not in the # new bundle. @@ -539,24 +569,28 @@ _install_multi_to_base() { [ -d "$existing" ] || continue _name="$(basename "$existing")" if [ ! -f "$_msrc/$_name/SKILL.md" ]; then - backup_and_remove_skill_dir "$existing" || true + backup_and_remove_skill_dir "$existing" || return 1 fi done if [ -d "$_base/dws-shared" ] && [ ! -f "$_msrc/dws-shared/SKILL.md" ]; then - backup_and_remove_skill_dir "$_base/dws-shared" || true + backup_and_remove_skill_dir "$_base/dws-shared" || return 1 fi + # Complete all backups before copying the first new skill so a later + # failure cannot leave a partial multi bundle in this Agent target. + for skill_dir in "$_msrc"/*/; do + [ -f "${skill_dir}SKILL.md" ] || continue + _name="$(basename "$skill_dir")" + _dest="$_base/$_name" + backup_and_remove_skill_dir "$_dest" || return 1 + done + _count=0 for skill_dir in "$_msrc"/*/; do [ -f "${skill_dir}SKILL.md" ] || continue _name="$(basename "$skill_dir")" _dest="$_base/$_name" - if [ -d "$_dest" ]; then - # Refreshing an existing skill: back it up first; on backup failure - # keep the user's copy and skip this skill. - backup_and_remove_skill_dir "$_dest" || continue - fi - mkdir -p "$_dest" + mkdir -p "$_dest" || return 1 cp -R "${skill_dir}." "$_dest/" 2>/dev/null || cp -r "${skill_dir}." "$_dest/" _count=$((_count + 1)) done @@ -577,7 +611,7 @@ _copy_skill_summary() { if [ -d "$_dest" ]; then backup_and_remove_skill_dir "$_dest" || { say " ⚠️ 跳过 ${_dest}(保留原目录)" - return 0 + return 1 } fi @@ -597,7 +631,7 @@ _copy_skill() { if [ -d "$_dest" ]; then backup_and_remove_skill_dir "$_dest" || { say " ⚠️ 跳过 ${_dest}(保留原目录)" - return 0 + return 1 } fi diff --git a/test/scripts/changelog_pr_gate_test.go b/test/scripts/changelog_pr_gate_test.go index d85a9d83..5335e57e 100644 --- a/test/scripts/changelog_pr_gate_test.go +++ b/test/scripts/changelog_pr_gate_test.go @@ -477,12 +477,13 @@ func TestChangelogPRFastPathWorkflowContract(t *testing.T) { "comparison.total_commits === comparison.ahead_by", "run.app?.slug !== 'github-actions'", "predecessor Code Admission is not fully successful", - `test "$(git rev-parse HEAD^1)" = "$PR_BASE_SHA"`, `test "$(git rev-parse HEAD^2)" = "$PR_HEAD_SHA"`, + `git merge-base --is-ancestor "$PR_BASE_SHA" "$merge_base_sha"`, + `echo "POLICY_BASE_SHA=$merge_base_sha"`, "Files API and synthetic merge tree disagree on CHANGELOG scope", "mode=--content-only", "mode=--fast-path", - `"$mode" "$PR_BASE_SHA" HEAD`, + `"$mode" "$POLICY_BASE_SHA" HEAD`, `test "$(git rev-parse HEAD)" = "$PUSH_AFTER_SHA"`, `--fast-path "$PUSH_BEFORE_SHA" "$PUSH_AFTER_SHA"`, "needs.lint.outputs.platform_sensitive == 'true'", @@ -515,8 +516,8 @@ func TestChangelogPRFastPathWorkflowContract(t *testing.T) { "name: Test (changed packages)", "changed-test-packages.sh", "Verify authoritative synthetic merge", - `test "$(git rev-parse HEAD^1)" = "$PR_BASE_SHA"`, `test "$(git rev-parse HEAD^2)" = "$PR_HEAD_SHA"`, + `echo "TEST_BASE_REF=$merge_base_sha"`, `echo "TEST_HEAD_REF=$(git rev-parse HEAD)"`, "list \"$TEST_BASE_REF\" \"$TEST_HEAD_REF\"", "needs.lint.outputs.full_suite != 'true'", diff --git a/test/scripts/install_js_smoke.mjs b/test/scripts/install_js_smoke.mjs index c8f76e76..c5746d17 100644 --- a/test/scripts/install_js_smoke.mjs +++ b/test/scripts/install_js_smoke.mjs @@ -23,6 +23,8 @@ * "invalid DWS_SKILL_MODE" error. * 4. multi-only zip, mono — mono install is skipped with a warning; the * staging root is NOT copied into a dws/ dir. + * 5. backup failure — preserves mono and writes no multi skill into + * the failed Agent target. * * Requirements: unix host with tar/zip/unzip on PATH (the same tools * install.js itself shells out to). Skips cleanly on win32. @@ -213,6 +215,29 @@ scenario("multi-only zip in mono mode skips skill install instead of copying sta } }); +scenario("backup failure preserves mono and writes no multi skills", () => { + const { tmp, pkg, home } = stagePkg({ + "mono/SKILL.md": "# mono fixture\n", + "multi/dingtalk-test/SKILL.md": "# dingtalk-test\n", + "multi/dws-shared/SKILL.md": "# dws-shared\n", + }); + try { + const base = path.join(home, ".agents", "skills"); + writeFile(path.join(base, "dws", "SKILL.md"), "old mono\n"); + // Poison the backup root: mkdirSync(/) must fail. + writeFile(path.join(home, ".dws", "skill-backups"), "not a directory\n"); + + const res = runInstall(pkg, home, "multi"); + assert.equal(res.status, 0, `exit=${res.status}\nstdout=${res.stdout}\nstderr=${res.stderr}`); + assert.match(res.stderr, /未安装 multi/); + assert.equal(fs.readFileSync(path.join(base, "dws", "SKILL.md"), "utf8"), "old mono\n"); + assert.ok(!fs.existsSync(path.join(base, "dingtalk-test")), "product skill not installed after cleanup failure"); + assert.ok(!fs.existsSync(path.join(base, "dws-shared")), "shared skill not installed after cleanup failure"); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); + for (const [name, fn] of scenarios) { process.stdout.write(`• ${name} ... `); fn(); diff --git a/test/scripts/install_script_test.go b/test/scripts/install_script_test.go index 08bae23b..b3d9498d 100644 --- a/test/scripts/install_script_test.go +++ b/test/scripts/install_script_test.go @@ -967,8 +967,8 @@ func findSkillBackup(fakeHome, name, content string) string { // TestInstallScriptBackupFailureKeepsOriginalDir pins the fail-safe contract: // when the backup destination cannot be created (skill-backups pre-created as // a regular file), the installer must keep every pre-existing skill directory -// untouched and still complete the install overall — a backup failure must -// never degrade into a hard delete or abort the run. +// untouched and skip the affected Agent target — a backup failure must never +// degrade into a hard delete or leave a mixed mono + multi layout. func TestInstallScriptBackupFailureKeepsOriginalDir(t *testing.T) { t.Parallel() @@ -1003,6 +1003,111 @@ func TestInstallScriptBackupFailureKeepsOriginalDir(t *testing.T) { if err != nil || string(data) != "old mono\n" { t.Fatalf("multi run must keep the mono leftover dws/ untouched on backup failure (data=%q, err=%v)\noutput:\n%s", string(data), err, out) } + for _, name := range []string{"dingtalk-test", "dws-shared"} { + if _, err := os.Stat(filepath.Join(base, name)); !os.IsNotExist(err) { + t.Fatalf("multi run must not install %s after cleanup backup failure, stat err=%v\noutput:\n%s", name, err, out) + } + } +} + +func TestInstallSkillsShellBackupFailureWritesNoMultiSkills(t *testing.T) { + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", "install-skills.sh")) + 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.Fatal("install-skills.sh final main invocation not found") + } + library := filepath.Join(t.TempDir(), "install-skills-lib.sh") + mustWriteFile(t, library, data[:cut], 0o755) + + home := t.TempDir() + root := filepath.Join(t.TempDir(), "project") + base := filepath.Join(root, ".agents", "skills") + multi := filepath.Join(t.TempDir(), "multi") + mustWriteFile(t, filepath.Join(base, "dws", "SKILL.md"), []byte("old mono\n"), 0o644) + mustWriteFile(t, filepath.Join(multi, "dingtalk-test", "SKILL.md"), []byte("new multi\n"), 0o644) + mustWriteFile(t, filepath.Join(home, ".dws", "skill-backups"), []byte("not a directory\n"), 0o644) + + harness := `. "$DWS_TEST_LIBRARY" +install_multi_skills_to_root "$DWS_TEST_MULTI" "$DWS_TEST_ROOT" +` + cmd := exec.Command("sh", "-c", harness) + cmd.Env = append(os.Environ(), + "HOME="+home, + "DWS_TEST_LIBRARY="+library, + "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) + } + 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) + } + if _, err := os.Stat(filepath.Join(base, "dingtalk-test")); !os.IsNotExist(err) { + t.Fatalf("multi installed after backup failure, stat err=%v", err) + } +} + +func TestInstallPowerShellBackupFailureWritesNoMultiSkills(t *testing.T) { + pwsh, err := exec.LookPath("pwsh") + if err != nil { + if runtime.GOOS == "windows" { + pwsh, err = exec.LookPath("powershell") + } + if err != nil { + t.Skip("PowerShell is not available") + } + } + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", "install.ps1")) + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(scriptPath) + if err != nil { + t.Fatal(err) + } + cut := strings.LastIndex(string(data), "# ── Main") + if cut < 0 { + t.Fatal("install.ps1 main section not found") + } + prefix := strings.ReplaceAll(string(data[:cut]), "$HOME", "$env:DWS_TEST_HOME") + prefix += ` +$ok = Install-MultiToBase -MultiSrc $env:DWS_TEST_MULTI -BaseDir $env:DWS_TEST_BASE -Root $env:DWS_TEST_HOME -AgentDir ".agents\skills" +if ($ok) { exit 2 } +exit 0 +` + harnessPath := filepath.Join(t.TempDir(), "install-harness.ps1") + mustWriteFile(t, harnessPath, []byte(prefix), 0o644) + + home := t.TempDir() + base := filepath.Join(home, ".agents", "skills") + multi := filepath.Join(t.TempDir(), "multi") + mustWriteFile(t, filepath.Join(base, "dws", "SKILL.md"), []byte("old mono\n"), 0o644) + mustWriteFile(t, filepath.Join(multi, "dingtalk-test", "SKILL.md"), []byte("new multi\n"), 0o644) + mustWriteFile(t, filepath.Join(home, ".dws", "skill-backups"), []byte("not a directory\n"), 0o644) + + cmd := exec.Command(pwsh, "-NoProfile", "-NonInteractive", "-File", harnessPath) + cmd.Env = append(os.Environ(), + "DWS_TEST_HOME="+home, + "DWS_TEST_MULTI="+multi, + "DWS_TEST_BASE="+base, + ) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("PowerShell harness failed: %v\n%s", err, output) + } + if data, err := os.ReadFile(filepath.Join(base, "dws", "SKILL.md")); err != nil || string(data) != "old mono\n" { + t.Fatalf("mono changed after PowerShell backup failure (data=%q, err=%v)", string(data), err) + } + if _, err := os.Stat(filepath.Join(base, "dingtalk-test")); !os.IsNotExist(err) { + t.Fatalf("PowerShell installed multi after backup failure, stat err=%v", err) + } } func writeTarGz(t *testing.T, path string, files map[string]string) {