From 62883b79406b7161302341539e625f32010cf057 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=89=E6=BE=9C?= Date: Sat, 15 Aug 2026 14:26:11 +0800 Subject: [PATCH] fix(skills): make obsolete-copy retirement non-fatal and harden install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A universal Agent whose obsolete private copy cannot be retired installs nothing there, yet every entry point counted that retirement failure as an install failure — aborting `npm install`, `dws skill setup`, and the shell installers even when the canonical store and all links published correctly, and skipping the skills-state write. Route retirement failures to a separate warning path across all surfaces (Go upgrade + skill setup, npm, PowerShell, install.sh, install-skills.sh, install-event.sh, install-devapp.sh). Also: - Add a checked-rename fallback for filesystems that reject the atomic no-replace flag (NFS, FUSE, overlayfs); the no-clobber contract is kept and the previously unsupported platforms build and work. - PowerShell multi-mode links only bundle skills, never the shared canonical store, so third-party/user skills are no longer fanned into every Agent root. - Prune ~/.dws/skill-backups to the newest 5 on every surface; encode HOME-relative backup names on PowerShell to preserve origin. - Add simulated-win32 junction coverage and rewrite the tautological no-replace test; remove dead code whose tests gave false coverage. - Soften the overstated Windows ownership-proof comment (NTFS tunneling). --- .changes/996-canonical-agent-skills.md | 2 +- build/npm/install.js | 72 ++++- docs/rfc-skill-installation-and-upgrade.md | 7 + internal/app/skill_setup.go | 70 +---- ...ll_setup_event_migration_universal_test.go | 13 +- internal/app/skill_setup_plan_test.go | 28 +- internal/app/upgrade.go | 3 + internal/upgrade/canonical_skills_test.go | 17 +- internal/upgrade/paths.go | 63 ++-- internal/upgrade/paths_multi_test.go | 42 ++- .../skill_publication_identity_windows.go | 8 +- internal/upgrade/skill_publish_rename.go | 40 +++ .../upgrade/skill_publish_rename_darwin.go | 17 +- .../upgrade/skill_publish_rename_linux.go | 18 +- .../upgrade/skill_publish_rename_other.go | 11 +- internal/upgrade/skill_publish_rename_test.go | 109 +++++++ .../upgrade/skill_publish_rename_windows.go | 7 +- scripts/install-devapp.ps1 | 76 ++++- scripts/install-devapp.sh | 81 ++++- scripts/install-event.sh | 85 +++++- scripts/install-skills.sh | 184 +++++++---- scripts/install.ps1 | 107 ++++++- scripts/install.sh | 190 ++++++++---- scripts/release/verify-package-managers.sh | 29 +- test/scripts/install_js_smoke.mjs | 285 ++++++++++++++++-- test/scripts/install_script_test.go | 15 +- test/scripts/package_script_test.go | 6 +- 27 files changed, 1266 insertions(+), 319 deletions(-) create mode 100644 internal/upgrade/skill_publish_rename.go create mode 100644 internal/upgrade/skill_publish_rename_test.go diff --git a/.changes/996-canonical-agent-skills.md b/.changes/996-canonical-agent-skills.md index b04fdd62..9a4afd7d 100644 --- a/.changes/996-canonical-agent-skills.md +++ b/.changes/996-canonical-agent-skills.md @@ -2,4 +2,4 @@ category: Fixed --- -- **Canonical Agent Skills** (#996) — installs bundled DWS Skills once under `~/.agents/skills`, migrates duplicate Agent copies, and matches the upstream 76-Agent registry across Go, npm, Shell, and PowerShell. Non-universal Agents use links (Windows junctions) with safe copy fallback; custom/XDG homes and OpenClaw legacy aliases are preserved. Upgrades now back up and restore Skills safely across external volumes by staging, lexically copying links (including dangling links), verifying contents, and deleting the source only after publication succeeds. Atomic no-replace publication and identity-checked quarantine rollback preserve concurrent user changes instead of overwriting or recursively deleting them. Standalone installers verify every downloaded release asset against `checksums.txt`, and the npm engine declaration now reflects the actual Node 16.7+ API floor. +- **Canonical Agent Skills** (#996) — installs bundled DWS Skills once under `~/.agents/skills`, migrates duplicate Agent copies, and matches the upstream 76-Agent registry across Go, npm, Shell, and PowerShell. Non-universal Agents use links (Windows junctions) with safe copy fallback; custom/XDG homes and OpenClaw legacy aliases are preserved. Upgrades now back up and restore Skills safely across external volumes by staging, lexically copying links (including dangling links), verifying contents, and deleting the source only after publication succeeds. Atomic no-replace publication and identity-checked quarantine rollback preserve concurrent user changes instead of overwriting or recursively deleting them; filesystems that reject the no-replace flag (NFS, FUSE, overlayfs) keep the no-clobber contract through a checked rename instead of failing the install. Failing to retire an obsolete Agent copy is now a warning rather than an install failure, and every install surface prunes `~/.dws/skill-backups` to the newest 5 batches. Standalone installers verify every downloaded release asset against `checksums.txt`, and the npm engine declaration now reflects the actual Node 16.7+ API floor. diff --git a/build/npm/install.js b/build/npm/install.js index d64e2ef0..1a243910 100755 --- a/build/npm/install.js +++ b/build/npm/install.js @@ -153,6 +153,42 @@ function backupStamp() { ); } +// SKILL_BACKUP_KEEP bounds ~/.dws/skill-backups/ growth: only the newest +// stamped backup directories are kept. Mirrors skillBackupKeep and +// pruneSkillBackups in internal/upgrade/paths.go so every installer surface +// applies the same retention. +const SKILL_BACKUP_KEEP = 5; + +// Backup roots this process created are never pruned: an in-flight +// transaction still needs them to roll back. +const currentRunBackupRoots = new Set(); + +// pruneSkillBackups removes the oldest stamped backup directories once more +// than SKILL_BACKUP_KEEP remain. Stamps sort lexicographically in +// chronological order (`YYYYmmdd-HHMMSS`), so name order is age order. +function pruneSkillBackups(homeDir) { + const root = path.join(homeDir, ".dws", "skill-backups"); + let entries; + try { + entries = fs.readdirSync(root, { withFileTypes: true }); + } catch (err) { + if (err && (err.code === "ENOENT" || err.code === "ENOTDIR")) return; + throw err; + } + const names = entries + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort((a, b) => Buffer.from(a).compare(Buffer.from(b))); + let excess = names.length - SKILL_BACKUP_KEEP; + for (const name of names) { + if (excess <= 0) break; + const candidate = path.join(root, name); + if (currentRunBackupRoots.has(candidate)) continue; + fs.rmSync(candidate, { recursive: true, force: true }); + excess -= 1; + } +} + function pathExistsLexicallySync(target) { try { fs.lstatSync(target); @@ -397,6 +433,14 @@ function backupAndRemoveSkillDir(homeDir, dir, backups = null, options = {}) { if (backups) { backups.push({ original: dir, backup: target }); } + currentRunBackupRoots.add(targetRoot); + // Keep the backup root bounded. The backup itself already succeeded, so a + // prune failure is only a warning (matches Go's best-effort prune call). + try { + pruneSkillBackups(homeDir); + } catch (err) { + console.warn(`⚠️ 旧备份清理失败(备份本身已成功): ${err.message}`); + } console.log(` × 已备份并移除 ${dir} → ${target}`); return target; } @@ -568,11 +612,14 @@ function installSkillsToHomes(skillRoot) { if (!agentTargetDetected(target) || samePhysicalDir(baseDir, canonicalBase)) continue; attempted += 1; if (universal) { + // Cleanup-only migration: universal Agents read the canonical store + // directly, so nothing is installed here. A retire failure leaves an + // obsolete copy in place — worth a warning, never a reason to fail an + // install whose canonical store and links all succeeded. try { retireManagedSkillRoot(homeDir, baseDir, managedNames); } catch (err) { - console.warn(`⚠️ Agent Skill 旧副本迁移失败 ${baseDir}: ${err.message}`); - failed += 1; + console.warn(`⚠️ Agent Skill 旧副本迁移失败(不影响本次安装)${baseDir}: ${err.message}`); } continue; } @@ -799,6 +846,15 @@ function rollbackPublishedSkillPath(publication, options = {}) { function publishCanonicalLinkNoReplace(staged, destination, options = {}) { const publication = captureSkillPublication(staged, destination); + // No-replace is a hard property of this publisher, so it is checked before + // either platform's publish call. Windows rename replaces the destination + // outright (libuv passes MOVEFILE_REPLACE_EXISTING) and `ln -P` treats an + // existing directory as a container to link INTO; in both cases the identity + // confirmation below compares against the staged object and could not + // recover the user's data. The sibling copy publishers guard identically. + if (skillPathExistsLexically(destination)) { + throw Object.assign(new Error(`Skill publish destination already exists: ${destination}`), { code: "EEXIST" }); + } if (process.platform === "win32") { const renameFn = options.publishRenameFn || fs.renameSync; renameFn(staged, destination); @@ -825,6 +881,9 @@ function publishCanonicalLinksAtomically(homeDir, canonicalBase, baseDir, names, const moveOptions = skillMoveOptions(options); const publishRemoveFn = options.publishRemoveFn || removeSkillPathLexically; const rollbackRenameFn = options.rollbackRenameFn || fs.renameSync; + // Injectable so the Windows junction type and absolute link target can be + // asserted from a non-Windows host. + const symlinkFn = options.symlinkFn || fs.symlinkSync; fs.mkdirSync(baseDir, { recursive: true }); const realBaseDir = fs.realpathSync(baseDir); const stageRoot = fs.mkdtempSync(path.join(baseDir, ".dws-link-set.tmp-")); @@ -864,7 +923,7 @@ function publishCanonicalLinksAtomically(homeDir, canonicalBase, baseDir, names, const stagedPath = path.join(stageRoot, name); const realTarget = fs.realpathSync(target); const linkTarget = process.platform === "win32" ? realTarget : path.relative(realBaseDir, realTarget); - fs.symlinkSync(linkTarget, stagedPath, process.platform === "win32" ? "junction" : "dir"); + symlinkFn(linkTarget, stagedPath, process.platform === "win32" ? "junction" : "dir"); staged.push({ staged: stagedPath, dest: path.join(baseDir, name) }); } const seen = new Set(); @@ -1181,11 +1240,14 @@ function installMultiSkillsToHomes(multiRoot) { if (!agentTargetDetected(target) || samePhysicalDir(baseDir, canonicalBase)) continue; attempted += 1; if (universal) { + // Cleanup-only migration (see installSkillsToHomes): nothing is + // installed in a universal root, so a retire failure must not fail an + // otherwise complete install. Go records the same case as a failed + // directory yet still returns success for the run. try { retireManagedSkillRoot(homeDir, baseDir, managedNames); } catch (err) { - console.warn(`⚠️ Agent Skill 旧副本迁移失败 ${baseDir}: ${err.message}`); - failed += 1; + console.warn(`⚠️ Agent Skill 旧副本迁移失败(不影响本次安装)${baseDir}: ${err.message}`); } continue; } diff --git a/docs/rfc-skill-installation-and-upgrade.md b/docs/rfc-skill-installation-and-upgrade.md index 56e70758..a2e90013 100644 --- a/docs/rfc-skill-installation-and-upgrade.md +++ b/docs/rfc-skill-installation-and-upgrade.md @@ -194,6 +194,13 @@ Agent 目标仍彼此独立,一个目标失败不会回滚此前已经成功 入口继续使用系统 `mv` 的跨文件系统复制/删除能力;Go、npm 与 PowerShell 显式实现 上述验证和失败合同。 +原子 no-replace 发布(Linux `RENAME_NOREPLACE`、Darwin `RENAME_EXCL`)依赖底层文件 +系统支持:`rename(2)` 只列出 ext4、btrfs、tmpfs 与 cifs,因此 NFS、FUSE 与 +overlayfs 家目录会以 `EINVAL` 拒绝该 flag。这些文件系统不得让安装整体失败,而是降级 +为“显式存在性检查 + 普通 rename”:不覆盖既有目标的契约保持不变,仅失去原子性, +其检查与 rename 之间的竞态窗口与普通 rename 本身一致,并由发布后的身份与内容校验 +兜底。Windows `MoveFile` 本身即拒绝已存在的目标,无需降级。 + 备份是安装安全机制,不等于独立 rollback 产品。需要切回 mono 时重新运行 `dws skill setup --mode mono`。 diff --git a/internal/app/skill_setup.go b/internal/app/skill_setup.go index 56858494..344809c7 100644 --- a/internal/app/skill_setup.go +++ b/internal/app/skill_setup.go @@ -388,7 +388,9 @@ func runSkillSetup(cmd *cobra.Command, _ []string) error { retiredNames = append(retiredNames, multiMiscSkill) } if retireErr := retireMigratedUniversalSkills(migrateEventMiscTargets, retiredNames, out); retireErr != nil { - return retireErr + // Retiring an obsolete universal copy installs nothing; report it and + // keep the successful installation rather than failing the run. + fmt.Fprintf(errOut, " ⚠️ %v\n", retireErr) } } if skipped > 0 { @@ -1231,63 +1233,6 @@ func samePhysicalSkillSetupPath(left, right string) bool { return leftErr == nil && rightErr == nil && sameSkillSetupPath(leftReal, rightReal) } -func genericSkillCleanupTarget(dests []string, managed map[string]bool) (*skillSetupTargetPlan, error) { - // Derive HOME from a concrete Agent destination instead of resolving it a - // second time. The destinations were already resolved from HOME by the - // caller, and a later/transient UserHomeDir failure must not turn an - // otherwise valid setup plan into an error. Direct/custom destinations that - // do not match a known concrete Agent root have no generic-root migration. - home := "" - for _, dest := range dests { - base := dest - if filepath.Base(dest) == "dws" { - base = filepath.Dir(dest) - } - base = filepath.Clean(base) - for i, rel := range skillSetupAgentHomes { - if i == 0 { - continue - } - suffix := filepath.Clean(filepath.FromSlash(rel)) - needle := string(filepath.Separator) + suffix - if strings.HasSuffix(base, needle) { - home = strings.TrimSuffix(base, needle) - break - } - } - if home != "" { - break - } - } - if home == "" { - return nil, nil - } - genericBase := filepath.Join(home, ".agents", "skills") - - target := &skillSetupTargetPlan{Destination: genericBase, CleanupOnly: true} - add := func(path, reason string) { - if info, statErr := skillSetupStat(path); statErr == nil && info.IsDir() { - target.Backups = append(target.Backups, skillSetupBackup{Path: path, Reason: reason}) - } - } - add(filepath.Join(genericBase, "dws"), skillSetupBackupMutual) - entries, readErr := skillSetupReadDir(genericBase) - if readErr != nil && !errors.Is(readErr, os.ErrNotExist) { - return nil, fmt.Errorf("扫描通用 Skill 根目录失败 %s: %w", genericBase, readErr) - } - for _, entry := range entries { - path := filepath.Join(genericBase, entry.Name()) - if entry.IsDir() && isManagedDWSMultiSkillDir(path, managed) { - target.Backups = append(target.Backups, skillSetupBackup{Path: path, Reason: skillSetupBackupStale}) - } - } - if len(target.Backups) == 0 { - return nil, nil - } - sort.Slice(target.Backups, func(i, j int) bool { return target.Backups[i].Path < target.Backups[j].Path }) - return target, nil -} - func buildSkillSetupPlan(mode, src string, dests, multiSkillNames []string, filtered bool) (*skillSetupPlan, error) { if mode != skillSetupModeMono && mode != skillSetupModeMulti { return nil, fmt.Errorf("内部错误:未知 mode %q", mode) @@ -2116,14 +2061,15 @@ func executeSkillSetupPlan(plan *skillSetupPlan, out, errOut io.Writer) (install } isCanonical := target.CanonicalBase != "" && sameSkillSetupPath(skillSetupBaseForMode(target.Destination, plan.Mode), target.CanonicalBase) if target.CleanupOnly { + // Nothing is installed below a universal root, so a stale copy that + // resists retirement must not count as a skipped install: any skipped + // count fails the whole setup, even when every real target succeeded. if homeErr != nil { - fmt.Fprintf(errOut, " ✗ 无法解析 HOME,保留 universal Agent 旧副本 %s: %v\n", target.Destination, homeErr) - skipped++ + fmt.Fprintf(errOut, " ⚠️ 无法解析 HOME,保留 universal Agent 旧副本 %s: %v\n", target.Destination, homeErr) continue } if _, cleanupErr := backupSkillSetupTarget(home, target.Backups, out); cleanupErr != nil { - fmt.Fprintf(errOut, " ✗ universal Agent 旧副本迁移失败,已回滚 %s: %v\n", target.Destination, cleanupErr) - skipped++ + fmt.Fprintf(errOut, " ⚠️ universal Agent 旧副本迁移失败,已回滚,可手动删除 %s: %v\n", target.Destination, cleanupErr) } continue } diff --git a/internal/app/skill_setup_event_migration_universal_test.go b/internal/app/skill_setup_event_migration_universal_test.go index 902aec6a..928f672d 100644 --- a/internal/app/skill_setup_event_migration_universal_test.go +++ b/internal/app/skill_setup_event_migration_universal_test.go @@ -112,7 +112,9 @@ func TestCrossPlatformCoverageSkillSetupUniversalRetirementFailures(t *testing.T } }) - t.Run("backup propagates through command", func(t *testing.T) { + // Retiring an obsolete universal copy installs nothing, so its failure is + // surfaced as a warning and the successful installation is kept. + t.Run("backup failure warns without failing the command", func(t *testing.T) { home := t.TempDir() testseam.Swap(t, &skillSetupUserHomeDir, func() (string, error) { return home, nil }) codex := filepath.Join(home, ".codex", "skills") @@ -121,14 +123,17 @@ func TestCrossPlatformCoverageSkillSetupUniversalRetirementFailures(t *testing.T testseam.Swap(t, &skillSetupBackupAndRemove, func(string, string) (string, error) { return "", failure }) - _, _, err := executeMultiSkillSetupTest( + _, stderr, err := executeMultiSkillSetupTest( t, src, []string{filepath.Join(home, ".agents", "skills"), codex}, "--skill", "event", "--yes", ) - if !errors.Is(err, failure) || !strings.Contains(err.Error(), "退役 universal Agent") { - t.Fatalf("retirement backup failure = %v, want wrapped %v", err, failure) + if err != nil { + t.Fatalf("retirement backup failure must not fail the command: %v", err) + } + if !strings.Contains(stderr, "退役 universal Agent") { + t.Fatalf("retirement backup failure must be reported, stderr = %q", stderr) } }) } diff --git a/internal/app/skill_setup_plan_test.go b/internal/app/skill_setup_plan_test.go index 406b318a..90b13869 100644 --- a/internal/app/skill_setup_plan_test.go +++ b/internal/app/skill_setup_plan_test.go @@ -174,32 +174,13 @@ func TestCrossPlatformCoverageSkillSetupGenericCleanupDerivesHomeFromConcreteTar t.Fatalf("universal cleanup preview missing: %s", preview.String()) } - t.Run("managed multi and scan failure", func(t *testing.T) { - genericMono := filepath.Join(home, ".agents", "skills", "dws") - if err := os.MkdirAll(genericMono, 0o755); err != nil { - t.Fatal(err) - } - managedDir := filepath.Join(home, ".agents", "skills", "dingtalk-chat") - if err := os.MkdirAll(managedDir, 0o755); err != nil { - t.Fatal(err) - } - target, targetErr := genericSkillCleanupTarget([]string{dest}, map[string]bool{"dingtalk-chat": true}) - if targetErr != nil || target == nil || len(target.Backups) != 2 { - t.Fatalf("managed generic cleanup = %#v, %v", target, targetErr) - } - failure := errors.New("generic scan failure") - testseam.Swap(t, &skillSetupReadDir, func(string) ([]os.DirEntry, error) { return nil, failure }) - if _, targetErr := genericSkillCleanupTarget([]string{dest}, nil); !errors.Is(targetErr, failure) { - t.Fatalf("generic scan error = %v", targetErr) - } - }) } func TestCrossPlatformCoverageSkillSetupCleanupOnlyExecutionBranches(t *testing.T) { failure := errors.New("cleanup failure") cleanup := skillSetupTargetPlan{Destination: "generic", CleanupOnly: true, Backups: []skillSetupBackup{{Path: "old"}}} - t.Run("prior skip suppresses cleanup", func(t *testing.T) { + t.Run("cleanup runs even after an install skip", func(t *testing.T) { testseam.Swap(t, &skillSetupUserHomeDir, func() (string, error) { return t.TempDir(), nil }) plan := &skillSetupPlan{Mode: skillSetupModeMono, Source: "missing", Targets: []skillSetupTargetPlan{{Destination: "install"}, cleanup}} installed, skipped, err := executeSkillSetupPlan(plan, io.Discard, io.Discard) @@ -208,11 +189,14 @@ func TestCrossPlatformCoverageSkillSetupCleanupOnlyExecutionBranches(t *testing. } }) + // A cleanup-only target installs nothing, so its failure is a warning and + // must never increment skipped — runSkillSetup turns any skipped count into + // a hard error and would fail an otherwise complete installation. t.Run("home failure keeps generic copy", func(t *testing.T) { testseam.Swap(t, &skillSetupUserHomeDir, func() (string, error) { return "", failure }) var stderr bytes.Buffer _, skipped, err := executeSkillSetupPlan(&skillSetupPlan{Mode: skillSetupModeMono, Targets: []skillSetupTargetPlan{cleanup}}, io.Discard, &stderr) - if err != nil || skipped != 1 || !strings.Contains(stderr.String(), "保留 universal Agent 旧副本") { + if err != nil || skipped != 0 || !strings.Contains(stderr.String(), "保留 universal Agent 旧副本") { t.Fatalf("cleanup HOME failure = (%d, %v, %q)", skipped, err, stderr.String()) } }) @@ -222,7 +206,7 @@ func TestCrossPlatformCoverageSkillSetupCleanupOnlyExecutionBranches(t *testing. testseam.Swap(t, &skillSetupBackupAndRemove, func(string, string) (string, error) { return "", failure }) var stderr bytes.Buffer _, skipped, err := executeSkillSetupPlan(&skillSetupPlan{Mode: skillSetupModeMono, Targets: []skillSetupTargetPlan{cleanup}}, io.Discard, &stderr) - if err != nil || skipped != 1 || !strings.Contains(stderr.String(), "迁移失败") { + if err != nil || skipped != 0 || !strings.Contains(stderr.String(), "迁移失败") { t.Fatalf("cleanup backup failure = (%d, %v, %q)", skipped, err, stderr.String()) } }) diff --git a/internal/app/upgrade.go b/internal/app/upgrade.go index 6388ce46..5c11dd5d 100644 --- a/internal/app/upgrade.go +++ b/internal/app/upgrade.go @@ -620,6 +620,9 @@ func runUpgrade(ctx context.Context, opts upgradeOptions) error { for _, d := range succeeded { fmt.Printf(" %s %s\n", ugDim("→"), ugCyan(shortenHome(d.Dir))) } + for _, d := range result.RetireWarnings() { + fmt.Printf(" %s %s %s\n", ugYellow("⚠"), shortenHome(d.Dir), ugDim("旧副本未能迁移,可手动删除: "+d.Err.Error())) + } } else { fmt.Printf(" %s\n", ugGreen("✓")) } diff --git a/internal/upgrade/canonical_skills_test.go b/internal/upgrade/canonical_skills_test.go index 9008fc31..174b776e 100644 --- a/internal/upgrade/canonical_skills_test.go +++ b/internal/upgrade/canonical_skills_test.go @@ -424,8 +424,21 @@ func TestCrossPlatformCoverageUpgradeAgentBranchFailures(t *testing.T) { return origRename(src, dst) }) result, err := UpgradeSkillLocations(makeSource(t)) - if err != nil || len(result.Failed()) != 1 { - t.Fatalf("retire failure = %#v, %v", result, err) + if err != nil { + t.Fatalf("retire failure must not fail the upgrade: %v", err) + } + if len(result.Failed()) != 0 { + t.Fatalf("retire failure must not count as an install failure: %#v", result.Failed()) + } + warnings := result.RetireWarnings() + if len(warnings) != 1 || warnings[0].Err == nil { + t.Fatalf("retire warnings = %#v", warnings) + } + if len(result.Succeeded()) == 0 { + t.Fatalf("canonical must still be published: %#v", result) + } + if _, statErr := os.Lstat(victim); statErr != nil { + t.Fatalf("unretired copy must be preserved: %v", statErr) } }) diff --git a/internal/upgrade/paths.go b/internal/upgrade/paths.go index 05551c79..f75d575d 100644 --- a/internal/upgrade/paths.go +++ b/internal/upgrade/paths.go @@ -239,13 +239,17 @@ const ( SkillDirSkipped // agent not detected, directory skipped SkillDirBlacklisted // blacklisted, never touched SkillDirFailed // installation attempted but failed + // SkillDirRetireWarning marks a universal Agent whose obsolete private copy + // could not be retired. Nothing is installed below such a root, so the + // leftover is reported without counting as an install failure. + SkillDirRetireWarning ) // SkillDirResult holds the per-directory install result. type SkillDirResult struct { Dir string // destination directory (e.g. ~/.claude/skills/dws) Status SkillDirStatus // outcome - Err error // non-nil when Status == SkillDirFailed + Err error // non-nil when Status == SkillDirFailed or SkillDirRetireWarning } // SkillUpgradeResult aggregates the outcome of an UpgradeSkillLocations call. @@ -275,6 +279,18 @@ func (r *SkillUpgradeResult) Failed() []SkillDirResult { return out } +// RetireWarnings returns universal Agent roots that still hold an obsolete +// private copy because retiring it failed. +func (r *SkillUpgradeResult) RetireWarnings() []SkillDirResult { + var out []SkillDirResult + for _, d := range r.Results { + if d.Status == SkillDirRetireWarning { + out = append(out, d) + } + } + return out +} + // UpgradeSkillLocations refreshes skills from extractedDir into agent homes. // extractedDir may be a multi-skill bundle root (subdirectories each containing // SKILL.md) or a legacy mono root (SKILL.md at its top level). Callers that @@ -789,7 +805,7 @@ func publishLinkedUpgradeTarget(homeDir, destBase, canonicalBase string, names [ } needed = append(needed, name) } - filteredVictims := victims[:0] + filteredVictims := make([]string, 0, len(victims)) for _, victim := range victims { if !correct[filepath.Clean(victim)] { filteredVictims = append(filteredVictims, victim) @@ -840,7 +856,9 @@ func upgradeMonoSkillLocations(homeDir, skillSrc string) (*SkillUpgradeResult, e } if root.universal { if err := retireManagedSkillRoot(homeDir, destBase, managedNames); err != nil { - result.Results = append(result.Results, SkillDirResult{Dir: destDir, Status: SkillDirFailed, Err: err}) + // Nothing is installed below a universal root, so a stale copy that + // resists retirement is a warning, not an install failure. + result.Results = append(result.Results, SkillDirResult{Dir: destDir, Status: SkillDirRetireWarning, Err: err}) } else { result.Results = append(result.Results, SkillDirResult{Dir: destDir, Status: SkillDirSkipped}) } @@ -912,7 +930,9 @@ func upgradeMultiSkillLocations(homeDir, multiRoot string, skills []string) (*Sk } if root.universal { if err := retireManagedSkillRoot(homeDir, destBase, managedNames); err != nil { - result.Results = append(result.Results, SkillDirResult{Dir: destBase, Status: SkillDirFailed, Err: err}) + // Nothing is installed below a universal root, so a stale copy that + // resists retirement is a warning, not an install failure. + result.Results = append(result.Results, SkillDirResult{Dir: destBase, Status: SkillDirRetireWarning, Err: err}) } else { result.Results = append(result.Results, SkillDirResult{Dir: destBase, Status: SkillDirSkipped}) } @@ -1005,23 +1025,6 @@ func skillTreeHasRoot(dir string) bool { return err == nil && !info.IsDir() } -// cleanupMultiLeftovers backs up + removes every proven DWS-managed -// multi-mode skill directory inside one agent home before mono is installed. -// A missing base directory simply means no leftovers; any other read failure -// is reported so mono never silently co-exists with multi. Removal is -// reversible: each leftover is preserved under ~/.dws/skill-backups/ and a -// backup failure aborts the removal for that home. -func cleanupMultiLeftovers(homeDir, baseDir string) error { - victims, err := managedMultiSkillVictims(baseDir, readManagedSkillNames(homeDir)) - if err != nil { - return err - } - if _, err := backupSkillSet(homeDir, victims); err != nil { - return fmt.Errorf("备份并清理 multi 残留失败: %w", err) - } - return nil -} - func managedMultiSkillVictims(baseDir string, managed ...map[string]bool) ([]string, error) { entries, err := upgradeReadDir(baseDir) if err != nil { @@ -1040,24 +1043,6 @@ func managedMultiSkillVictims(baseDir string, managed ...map[string]bool) ([]str return victims, nil } -// cleanupOppositeModeLeftovers backs up + removes, inside one agent home, the -// legacy mono directory (dws/) and every proven DWS-managed multi skill -// directory that is not part of the new bundle. A dingtalk-* prefix alone is -// not proof of ownership because market/user skills may use the same prefix. -// Removal is reversible: each -// directory is preserved under ~/.dws/skill-backups/ and a backup failure -// aborts the removal for that home. -func cleanupOppositeModeLeftovers(homeDir, destBase string, skillSet map[string]bool) error { - victims, err := oppositeModeSkillVictims(destBase, skillSet, readManagedSkillNames(homeDir)) - if err != nil { - return err - } - if _, err := backupSkillSet(homeDir, victims); err != nil { - return fmt.Errorf("备份并清理对面模式残留失败: %w", err) - } - return nil -} - func oppositeModeSkillVictims(destBase string, skillSet map[string]bool, managed ...map[string]bool) ([]string, error) { victims := []string{filepath.Join(destBase, "dws")} entries, err := upgradeReadDir(destBase) diff --git a/internal/upgrade/paths_multi_test.go b/internal/upgrade/paths_multi_test.go index 6ba5d481..9ae6579d 100644 --- a/internal/upgrade/paths_multi_test.go +++ b/internal/upgrade/paths_multi_test.go @@ -994,6 +994,30 @@ func TestCrossPlatformCoverageMultiUpgradeBackupAndFallbackEdges(t *testing.T) { } } +// The production wrappers that used to bundle these two steps were removed as +// dead code; the test keeps exercising the live building blocks they composed. +func cleanupMultiLeftoversForTest(homeDir, baseDir string) error { + victims, err := managedMultiSkillVictims(baseDir, readManagedSkillNames(homeDir)) + if err != nil { + return err + } + if _, err := backupSkillSet(homeDir, victims); err != nil { + return fmt.Errorf("备份并清理 multi 残留失败: %w", err) + } + return nil +} + +func cleanupOppositeModeLeftoversForTest(homeDir, destBase string, skillSet map[string]bool) error { + victims, err := oppositeModeSkillVictims(destBase, skillSet, readManagedSkillNames(homeDir)) + if err != nil { + return err + } + if _, err := backupSkillSet(homeDir, victims); err != nil { + return fmt.Errorf("备份并清理对面模式残留失败: %w", err) + } + return nil +} + // TestCrossPlatformCoverageCleanupLeftoversEdges pins the cleanup helpers: // read failures surface, backup failures abort, and the opposite-mode cleanup // preserves bundle skills while removing the mono leftover. @@ -1003,11 +1027,11 @@ func TestCrossPlatformCoverageCleanupLeftoversEdges(t *testing.T) { // Read failure (non-ENOENT) surfaces from both cleanups. testseam.Swap(t, &upgradeReadDir, func(string) ([]os.DirEntry, error) { return nil, errors.New("read denied") }) - if err := cleanupMultiLeftovers(home, base); err == nil || !strings.Contains(err.Error(), "读取技能目录失败") { - t.Fatalf("cleanupMultiLeftovers read error = %v", err) + if err := cleanupMultiLeftoversForTest(home, base); err == nil || !strings.Contains(err.Error(), "读取技能目录失败") { + t.Fatalf("cleanupMultiLeftoversForTest read error = %v", err) } - if err := cleanupOppositeModeLeftovers(home, base, map[string]bool{}); err == nil || !strings.Contains(err.Error(), "读取技能目录失败") { - t.Fatalf("cleanupOppositeModeLeftovers read error = %v", err) + if err := cleanupOppositeModeLeftoversForTest(home, base, map[string]bool{}); err == nil || !strings.Contains(err.Error(), "读取技能目录失败") { + t.Fatalf("cleanupOppositeModeLeftoversForTest read error = %v", err) } testseam.Swap(t, &upgradeReadDir, os.ReadDir) @@ -1015,13 +1039,13 @@ func TestCrossPlatformCoverageCleanupLeftoversEdges(t *testing.T) { os.MkdirAll(filepath.Join(base, "dingtalk-stale"), 0o755) useUpgradeManagedNames(t, "dingtalk-stale") testseam.Swap(t, &skillPathRenameNoReplace, func(string, string) error { return errors.New("backup denied") }) - if err := cleanupMultiLeftovers(home, base); err == nil || !strings.Contains(err.Error(), "备份并清理 multi 残留失败") { + if err := cleanupMultiLeftoversForTest(home, base); err == nil || !strings.Contains(err.Error(), "备份并清理 multi 残留失败") { t.Fatalf("cleanupMultiLeftovers backup error = %v", err) } // Backup failure of the mono leftover aborts the opposite-mode cleanup. os.MkdirAll(filepath.Join(base, "dws"), 0o755) - if err := cleanupOppositeModeLeftovers(home, base, map[string]bool{}); err == nil || !strings.Contains(err.Error(), "备份并清理对面模式残留失败") { + if err := cleanupOppositeModeLeftoversForTest(home, base, map[string]bool{}); err == nil || !strings.Contains(err.Error(), "备份并清理对面模式残留失败") { t.Fatalf("opposite cleanup mono backup error = %v", err) } testseam.Swap(t, &skillPathRenameNoReplace, renameSkillPathNoReplace) @@ -1033,7 +1057,7 @@ func TestCrossPlatformCoverageCleanupLeftoversEdges(t *testing.T) { } return renameSkillPathNoReplace(src, dst) }) - if err := cleanupOppositeModeLeftovers(home, base, map[string]bool{}); err == nil || !strings.Contains(err.Error(), "备份并清理对面模式残留失败") { + if err := cleanupOppositeModeLeftoversForTest(home, base, map[string]bool{}); err == nil || !strings.Contains(err.Error(), "备份并清理对面模式残留失败") { t.Fatalf("opposite cleanup stale backup error = %v", err) } testseam.Swap(t, &skillPathRenameNoReplace, renameSkillPathNoReplace) @@ -1044,7 +1068,7 @@ func TestCrossPlatformCoverageCleanupLeftoversEdges(t *testing.T) { skillSet := map[string]bool{"dingtalk-keep": true} os.MkdirAll(filepath.Join(base, "dingtalk-keep"), 0o755) os.WriteFile(filepath.Join(base, "regular-file"), []byte("x"), 0o644) - if err := cleanupOppositeModeLeftovers(home, base, skillSet); err != nil { + if err := cleanupOppositeModeLeftoversForTest(home, base, skillSet); err != nil { t.Fatalf("cleanupOppositeModeLeftovers() error = %v", err) } for _, gone := range []string{"dws", "dingtalk-stale"} { @@ -1064,7 +1088,7 @@ func TestCrossPlatformCoverageCleanupLeftoversEdges(t *testing.T) { } // cleanupMultiLeftovers on a missing base is a no-op. - if err := cleanupMultiLeftovers(home, filepath.Join(home, "missing")); err != nil { + if err := cleanupMultiLeftoversForTest(home, filepath.Join(home, "missing")); err != nil { t.Fatalf("missing base must be a no-op, got %v", err) } } diff --git a/internal/upgrade/skill_publication_identity_windows.go b/internal/upgrade/skill_publication_identity_windows.go index 03c97de7..107e266a 100644 --- a/internal/upgrade/skill_publication_identity_windows.go +++ b/internal/upgrade/skill_publication_identity_windows.go @@ -18,7 +18,11 @@ func skillPathFileIncarnation(info os.FileInfo) string { func skillPathSameFileIdentity(_, _ os.FileInfo) bool { // MoveFile can surface different metadata identities for the same reparse - // point before and after rename. Windows ownership is therefore proved by - // the separately checked creation-time incarnation plus lexical fingerprint. + // point before and after rename, so Windows ownership relies on the + // separately checked creation-time incarnation plus the lexical fingerprint. + // That pair is strong evidence but not proof: NTFS file tunneling restores + // the original creation time when a same-named object is recreated in the + // same directory shortly afterwards, so a byte-identical concurrent + // replacement can still be attributed to this transaction. return true } diff --git a/internal/upgrade/skill_publish_rename.go b/internal/upgrade/skill_publish_rename.go new file mode 100644 index 00000000..490f0f9a --- /dev/null +++ b/internal/upgrade/skill_publish_rename.go @@ -0,0 +1,40 @@ +package upgrade + +import ( + "errors" + "fmt" + "os" +) + +// errNoReplaceRenameUnsupported marks a platform that has no atomic no-replace +// rename primitive at all. +var errNoReplaceRenameUnsupported = errors.New("平台不支持原子 no-replace rename") + +var skillPathRenameNoReplaceAtomic = renameSkillPathNoReplaceAtomic + +// renameSkillPathNoReplace moves source onto destination and never replaces an +// object that already occupies destination. +// +// The atomic primitives (Linux RENAME_NOREPLACE, Darwin RENAME_EXCL) require +// support from the underlying filesystem and report EINVAL/ENOTSUP when it is +// missing — rename(2) lists only ext4, btrfs, tmpfs and cifs, so NFS, FUSE and +// overlayfs homes reject the flag. Failing there would abort the whole install +// on those filesystems, so they degrade to an explicit existence check plus a +// plain rename: the no-clobber contract is preserved, only its atomicity is +// lost. That check-then-rename window is the same exposure a plain rename +// always had, and callers additionally verify identity after publication. +func renameSkillPathNoReplace(source, destination string) error { + err := skillPathRenameNoReplaceAtomic(source, destination) + if err == nil { + return nil + } + if !errors.Is(err, errNoReplaceRenameUnsupported) && !isNoReplaceRenameUnsupported(err) { + return err + } + if _, statErr := skillPathLstat(destination); statErr == nil { + return &os.LinkError{Op: "rename", Old: source, New: destination, Err: os.ErrExist} + } else if !os.IsNotExist(statErr) { + return fmt.Errorf("检查发布目标失败 %s: %w", destination, statErr) + } + return skillPathRename(source, destination) +} diff --git a/internal/upgrade/skill_publish_rename_darwin.go b/internal/upgrade/skill_publish_rename_darwin.go index 06722ef1..94df0fb2 100644 --- a/internal/upgrade/skill_publish_rename_darwin.go +++ b/internal/upgrade/skill_publish_rename_darwin.go @@ -2,8 +2,21 @@ package upgrade -import "golang.org/x/sys/unix" +import ( + "errors" + "syscall" -func renameSkillPathNoReplace(source, destination string) error { + "golang.org/x/sys/unix" +) + +func renameSkillPathNoReplaceAtomic(source, destination string) error { return unix.RenamexNp(source, destination, unix.RENAME_EXCL) } + +// Volumes whose filesystem does not implement renamex_np (several network and +// FUSE mounts) reject RENAME_EXCL instead of performing the rename. +func isNoReplaceRenameUnsupported(err error) bool { + return errors.Is(err, syscall.ENOTSUP) || + errors.Is(err, syscall.EINVAL) || + errors.Is(err, syscall.ENOSYS) +} diff --git a/internal/upgrade/skill_publish_rename_linux.go b/internal/upgrade/skill_publish_rename_linux.go index 7936592a..20d1be77 100644 --- a/internal/upgrade/skill_publish_rename_linux.go +++ b/internal/upgrade/skill_publish_rename_linux.go @@ -2,8 +2,22 @@ package upgrade -import "golang.org/x/sys/unix" +import ( + "errors" + "syscall" -func renameSkillPathNoReplace(source, destination string) error { + "golang.org/x/sys/unix" +) + +func renameSkillPathNoReplaceAtomic(source, destination string) error { return unix.Renameat2(unix.AT_FDCWD, source, unix.AT_FDCWD, destination, unix.RENAME_NOREPLACE) } + +// rename(2) documents EINVAL as "the filesystem does not support one of the +// flags", which is what NFS, FUSE and overlayfs return; ENOSYS covers kernels +// without renameat2 at all. +func isNoReplaceRenameUnsupported(err error) bool { + return errors.Is(err, syscall.EINVAL) || + errors.Is(err, syscall.ENOSYS) || + errors.Is(err, syscall.EOPNOTSUPP) +} diff --git a/internal/upgrade/skill_publish_rename_other.go b/internal/upgrade/skill_publish_rename_other.go index bdd0986b..c9695771 100644 --- a/internal/upgrade/skill_publish_rename_other.go +++ b/internal/upgrade/skill_publish_rename_other.go @@ -2,11 +2,8 @@ package upgrade -import ( - "fmt" - "runtime" -) - -func renameSkillPathNoReplace(_, _ string) error { - return fmt.Errorf("当前平台 %s 不支持安全的原子 no-replace rename", runtime.GOOS) +func renameSkillPathNoReplaceAtomic(_, _ string) error { + return errNoReplaceRenameUnsupported } + +func isNoReplaceRenameUnsupported(error) bool { return false } diff --git a/internal/upgrade/skill_publish_rename_test.go b/internal/upgrade/skill_publish_rename_test.go new file mode 100644 index 00000000..e3996aa3 --- /dev/null +++ b/internal/upgrade/skill_publish_rename_test.go @@ -0,0 +1,109 @@ +package upgrade + +import ( + "errors" + "os" + "path/filepath" + "syscall" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" +) + +// TestCrossPlatformCoverageNoReplaceRenameFallback pins the degradation path for +// filesystems that reject RENAME_NOREPLACE / RENAME_EXCL (NFS, FUSE, overlayfs). +// Those installs must still publish, and must still refuse to clobber an +// occupied destination. +func TestCrossPlatformCoverageNoReplaceRenameFallback(t *testing.T) { + unsupported := []error{syscall.EINVAL, syscall.ENOSYS, errNoReplaceRenameUnsupported} + + t.Run("publishes when the filesystem rejects the flag", func(t *testing.T) { + for _, unsupportedErr := range unsupported { + dir := t.TempDir() + source := filepath.Join(dir, "source") + destination := filepath.Join(dir, "destination") + if err := os.WriteFile(source, []byte("payload"), 0o644); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &skillPathRenameNoReplaceAtomic, func(string, string) error { return unsupportedErr }) + if err := renameSkillPathNoReplace(source, destination); err != nil { + t.Fatalf("fallback must publish for %v, got %v", unsupportedErr, err) + } + if data, err := os.ReadFile(destination); err != nil || string(data) != "payload" { + t.Fatalf("destination = %q, %v", data, err) + } + if _, err := os.Lstat(source); !os.IsNotExist(err) { + t.Fatalf("source must be gone, stat err=%v", err) + } + } + }) + + t.Run("still refuses an occupied destination", func(t *testing.T) { + dir := t.TempDir() + source := filepath.Join(dir, "source") + destination := filepath.Join(dir, "destination") + for _, path := range []string{source, destination} { + if err := os.WriteFile(path, []byte(filepath.Base(path)), 0o644); err != nil { + t.Fatal(err) + } + } + testseam.Swap(t, &skillPathRenameNoReplaceAtomic, func(string, string) error { return syscall.EINVAL }) + err := renameSkillPathNoReplace(source, destination) + if !errors.Is(err, os.ErrExist) { + t.Fatalf("occupied destination must report ErrExist, got %v", err) + } + if data, readErr := os.ReadFile(destination); readErr != nil || string(data) != "destination" { + t.Fatalf("destination must be untouched, got %q, %v", data, readErr) + } + if _, statErr := os.Lstat(source); statErr != nil { + t.Fatalf("source must be preserved: %v", statErr) + } + }) + + t.Run("a genuine error is not retried", func(t *testing.T) { + dir := t.TempDir() + source := filepath.Join(dir, "source") + if err := os.WriteFile(source, []byte("payload"), 0o644); err != nil { + t.Fatal(err) + } + calls := 0 + testseam.Swap(t, &skillPathRenameNoReplaceAtomic, func(string, string) error { + calls++ + return os.ErrPermission + }) + if err := renameSkillPathNoReplace(source, filepath.Join(dir, "destination")); !errors.Is(err, os.ErrPermission) { + t.Fatalf("permission error must surface, got %v", err) + } + if calls != 1 { + t.Fatalf("atomic attempts = %d, want 1", calls) + } + }) + + t.Run("cross-device errors still reach the copy fallback", func(t *testing.T) { + if isNoReplaceRenameUnsupported(syscall.EXDEV) { + t.Fatal("EXDEV must not be treated as an unsupported flag") + } + dir := t.TempDir() + source := filepath.Join(dir, "source") + if err := os.MkdirAll(source, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "SKILL.md"), []byte("payload"), 0o644); err != nil { + t.Fatal(err) + } + destination := filepath.Join(dir, "backup", "destination") + original := skillPathRenameNoReplaceAtomic + testseam.Swap(t, &skillPathRenameNoReplaceAtomic, func(from, to string) error { + if from == source { + return syscall.EXDEV + } + return original(from, to) + }) + if err := moveSkillPathRecoverably(source, destination); err != nil { + t.Fatalf("cross-device move = %v", err) + } + if data, err := os.ReadFile(filepath.Join(destination, "SKILL.md")); err != nil || string(data) != "payload" { + t.Fatalf("cross-device destination = %q, %v", data, err) + } + }) +} diff --git a/internal/upgrade/skill_publish_rename_windows.go b/internal/upgrade/skill_publish_rename_windows.go index 7ac5afb1..18927afe 100644 --- a/internal/upgrade/skill_publish_rename_windows.go +++ b/internal/upgrade/skill_publish_rename_windows.go @@ -4,7 +4,7 @@ package upgrade import "golang.org/x/sys/windows" -func renameSkillPathNoReplace(source, destination string) error { +func renameSkillPathNoReplaceAtomic(source, destination string) error { sourcePtr, err := windows.UTF16PtrFromString(source) if err != nil { return err @@ -15,3 +15,8 @@ func renameSkillPathNoReplace(source, destination string) error { } return windows.MoveFile(sourcePtr, destinationPtr) } + +// MoveFile without MOVEFILE_REPLACE_EXISTING already refuses an occupied +// destination on every supported Windows version, so there is nothing to +// degrade to. +func isNoReplaceRenameUnsupported(error) bool { return false } diff --git a/scripts/install-devapp.ps1 b/scripts/install-devapp.ps1 index 1719391d..50502caa 100644 --- a/scripts/install-devapp.ps1 +++ b/scripts/install-devapp.ps1 @@ -258,15 +258,73 @@ function Move-DevSkillPathRecoverably([string]$Source, [string]$Destination) { } } +# Get-DevSkillBackupName encodes the HOME-relative path of a backed-up Skill +# directory ('.codex\skills\dingtalk-misc' → '.codex-skills-dingtalk-misc') so +# copies retired from different Agent roots stay distinguishable. Mirrors +# build/npm/install.js and internal/upgrade/paths.go; paths outside HOME fall +# back to the bare leaf. +function Get-DevSkillBackupName([string]$Dir) { + $leaf = Split-Path $Dir -Leaf + try { + $full = [System.IO.Path]::GetFullPath($Dir) + $root = [System.IO.Path]::GetFullPath($HOME).TrimEnd([char[]]@('\', '/')) + } catch { return $leaf } + if ([string]::IsNullOrWhiteSpace($root)) { return $leaf } + foreach ($sep in @('\', '/')) { + $prefix = $root + $sep + if ($full.Length -gt $prefix.Length -and + $full.StartsWith($prefix, [System.StringComparison]::OrdinalIgnoreCase)) { + $rel = $full.Substring($prefix.Length).Trim([char[]]@('\', '/')) + if (![string]::IsNullOrWhiteSpace($rel)) { return ($rel -replace '[\\/]+', '-') } + } + } + return $leaf +} + +# Keep $HOME\.dws\skill-backups bounded to the newest stamped roots, matching +# skillBackupKeep / pruneSkillBackups in internal/upgrade/paths.go. Roots this +# run created are never pruned: a rollback still needs them. +$DevSkillBackupKeep = 5 +$script:DevSkillBackupRootsThisRun = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) + +function Remove-OldDevSkillBackups { + $root = Join-Path $HOME ".dws\skill-backups" + if (!(Test-Path -LiteralPath $root -PathType Container)) { return } + $dirs = @(Get-ChildItem -LiteralPath $root -Directory -Force -ErrorAction SilentlyContinue | + Sort-Object -Property Name) + $excess = $dirs.Count - $DevSkillBackupKeep + foreach ($dir in $dirs) { + if ($excess -le 0) { break } + if ($script:DevSkillBackupRootsThisRun.Contains($dir.FullName)) { continue } + Remove-Item -LiteralPath $dir.FullName -Recurse -Force -ErrorAction SilentlyContinue + $excess-- + } +} + function Backup-DevSkill([string]$path, [ref]$BackupPath) { if ($null -ne $BackupPath) { $BackupPath.Value = "" } if (!(Test-PathLexically $path)) { return } - $backupRoot = Join-Path $HOME ".dws\skill-backups\$([DateTime]::UtcNow.ToString('yyyyMMdd-HHmmss'))" + $backupBase = Join-Path $HOME ".dws\skill-backups" + $stamp = [DateTime]::UtcNow.ToString('yyyyMMdd-HHmmss') + $name = Get-DevSkillBackupName $path + $backupRoot = Join-Path $backupBase $stamp + $target = Join-Path $backupRoot $name + $i = 1 + while (Test-PathLexically $target) { + $backupRoot = Join-Path $backupBase "$stamp-$i" + $target = Join-Path $backupRoot $name + $i++ + if ($i -gt 1000) { throw "备份目录冲突无法解决,保留原目录: $path" } + } New-Item -ItemType Directory -Path $backupRoot -Force -ErrorAction Stop | Out-Null - $target = Join-Path $backupRoot (Split-Path $path -Leaf) - while (Test-PathLexically $target) { $target += "-$([guid]::NewGuid().ToString('N'))" } Move-DevSkillPathRecoverably $path $target if ($null -ne $BackupPath) { $BackupPath.Value = $target } + try { + $script:DevSkillBackupRootsThisRun.Add([System.IO.Path]::GetFullPath($backupRoot)) | Out-Null + Remove-OldDevSkillBackups + } catch { + Say "⚠️ 旧备份清理失败(备份本身已成功): $($_.Exception.Message)" + } } function Get-DevSkillLinkSignature([string]$Path) { @@ -487,6 +545,7 @@ if (-not $NoSkills) { Publish-DevSkillCopy $src $canonical $installed = 1 $failed = 0 + $retireFailed = 0 $seen = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) foreach ($row in @($AgentRegistryRows) + @($LegacyAgentCleanupRows)) { $parts = $row.Split('|'); $universal = $parts[1] -eq "1" @@ -497,9 +556,13 @@ if (-not $NoSkills) { if (!(Test-AgentBaseDetected $parts[0] $base)) { continue } $dest = Join-Path $base $SkillName if ($universal) { + # Cleanup-only migration: universal Agents read the + # canonical store directly, so nothing is installed here. A + # retire failure only leaves an obsolete copy behind and + # must never fail an otherwise complete install. try { Backup-DevSkill $dest } catch { - Say "⚠️ Agent Skill 旧副本备份失败,保留原目录: $dest ($($_.Exception.Message))" - $failed++ + Say "⚠️ Agent Skill 旧副本备份失败,保留原目录(不影响本次安装): $dest ($($_.Exception.Message))" + $retireFailed++ } continue } @@ -523,6 +586,9 @@ if (-not $NoSkills) { $installed++ } if ($failed -gt 0) { Die "有 $failed 个 Agent 目标安装 $SkillName 失败(其余目标已安装)" } + if ($retireFailed -gt 0) { + Say "⚠️ 有 $retireFailed 个 Agent 旧副本未能迁移(安装已完成,可稍后手动删除)" + } Say "Skill dingtalk-misc -> $installed agent dir(s)" } else { Say "(dingtalk-misc not found in skills bundle; skipped)" diff --git a/scripts/install-devapp.sh b/scripts/install-devapp.sh index 698d9048..e2a9219e 100755 --- a/scripts/install-devapp.sh +++ b/scripts/install-devapp.sh @@ -62,18 +62,84 @@ copy_tree() { return 1 } +# refresh_cache_tree +# Ephemeral ~/.dws/skills cache refresh, identical to install-event.sh. The tree +# is byte-reproducible from the release asset, so it is never archived into +# ~/.dws/skill-backups; unlike the previous `rm -rf` + `cp` it only deletes the +# old cache after the replacement is fully staged, so an interrupted install can +# no longer leave a half-copied cache behind. +refresh_cache_tree() { + src="$1"; dest="$2"; parent="$(dirname "$dest")" + mkdir -p "$parent" || return 1 + stage="$(mktemp -d "$parent/.dws-cache.tmp.XXXXXX")" || return 1 + if ! cp -R "$src/." "$stage/"; then rm -rf "$stage"; return 1; fi + old="" + if [ -e "$dest" ] || [ -L "$dest" ]; then + old="$(mktemp -d "$parent/.dws-cache.old.XXXXXX")" || { rm -rf "$stage"; return 1; } + rmdir "$old" || { rm -rf "$stage" "$old"; return 1; } + if ! mv "$dest" "$old"; then rm -rf "$stage"; return 1; fi + fi + if mv "$stage" "$dest"; then + if [ -n "$old" ] && ! rm -rf "$old"; then + printf ' ⚠️ 新缓存已生效,但旧缓存清理失败: %s\n' "$old" >&2 + fi + return 0 + fi + rm -rf "$stage" + if [ -n "$old" ] && ! mv "$old" "$dest"; then + printf ' ⚠️ 缓存刷新失败,原缓存保留在 %s\n' "$old" >&2 + fi + return 1 +} + backup_skill_dir() { victim="$1" [ -e "$victim" ] || [ -L "$victim" ] || { printf '\n'; return 0; } + # Prune before creating this run's archive, so the backup the caller is about + # to depend on for rollback can never be a prune candidate. + prune_skill_backups stamp="$(date -u +%Y%m%d-%H%M%S)" name="$(printf '%s' "$victim" | sed 's#[/\\]#-#g; s#^-##')" backup_root="$HOME/.dws/skill-backups/$stamp"; backup="$backup_root/$name"; i=0 - while [ -e "$backup" ] || [ -L "$backup" ]; do i=$((i + 1)); backup_root="$HOME/.dws/skill-backups/$stamp-$i"; backup="$backup_root/$name"; done + while [ -e "$backup" ] || [ -L "$backup" ]; do + i=$((i + 1)); backup_root="$HOME/.dws/skill-backups/$stamp-$i"; backup="$backup_root/$name" + # Same bail-out as install-skills.sh: never spin forever on a pathological + # backup root, report it and keep the original directory instead. + if [ "$i" -gt 1000 ]; then + printf ' ⚠️ 备份目录冲突,保留原目录 %s\n' "$victim" >&2 + return 1 + fi + done mkdir -p "$backup_root" || return 1 mv "$victim" "$backup" || return 1 printf '%s\n' "$backup" } +# prune_skill_backups keeps only the newest SKILL_BACKUP_KEEP stamped backup +# directories under $HOME/.dws/skill-backups, matching Go's skillBackupKeep / +# pruneSkillBackups. Glob order is lexicographic and the stamps are UTC +# YYYYmmdd-HHMMSS, so the oldest sort first. Best-effort: a removal failure is +# reported and never fatal. +SKILL_BACKUP_KEEP=5 +prune_skill_backups() { + prune_root="$HOME/.dws/skill-backups" + [ -d "$prune_root" ] || return 0 + prune_total=0 + for prune_entry in "$prune_root"/*; do + [ -d "$prune_entry" ] || continue + prune_total=$((prune_total + 1)) + done + [ "$prune_total" -gt "$SKILL_BACKUP_KEEP" ] || return 0 + prune_drop=$((prune_total - SKILL_BACKUP_KEEP)) + prune_seen=0 + for prune_entry in "$prune_root"/*; do + [ -d "$prune_entry" ] || continue + prune_seen=$((prune_seen + 1)) + [ "$prune_seen" -le "$prune_drop" ] || break + rm -rf "$prune_entry" || printf ' ⚠️ 旧 Skill 备份清理失败: %s\n' "$prune_entry" >&2 + done +} + same_physical_skill() { [ -d "$1" ] && [ -d "$2" ] || return 1 left="$(CDPATH= cd -- "$1" 2>/dev/null && pwd -P)" || return 1 @@ -200,20 +266,24 @@ install_skill() { # cache so `dws skill setup --mode multi` can find a source later cache="$HOME/.dws/skills/multi/$SKILL_NAME" - rm -rf "$cache"; mkdir -p "$cache"; cp -R "$src/." "$cache/" + refresh_cache_tree "$src" "$cache" || { printf ' ⚠️ Skill 缓存刷新失败: %s\n' "$cache" >&2; return 1; } canonical="$HOME/.agents/skills/$SKILL_NAME" copy_tree "$src" "$canonical" || return 1 installed=1 failed=0 + retire_failed=0 for agent_dir in $(agent_skill_dirs); do base="$(resolve_agent_skill_base "$agent_dir")" agent_skill_base_detected "$agent_dir" "$base" || continue same_physical_skill "$base" "$HOME/.agents/skills" && continue if is_cleanup_only_agent_dir "$agent_dir"; then + # Cleanup-only: universal Agents read the canonical store directly, so + # nothing is installed here. A retire failure only leaves an obsolete copy + # behind and must never fail an otherwise complete install. if ! backup_skill_dir "$base/$SKILL_NAME" >/dev/null; then - printf ' ⚠️ Agent Skill 旧副本备份失败,保留原目录: %s\n' "$base/$SKILL_NAME" >&2 - failed=$((failed + 1)) + printf ' ⚠️ Agent Skill 旧副本备份失败,保留原目录(不影响本次安装): %s\n' "$base/$SKILL_NAME" >&2 + retire_failed=$((retire_failed + 1)) fi continue fi @@ -226,6 +296,9 @@ install_skill() { fi installed=$((installed + 1)) done + if [ "$retire_failed" -gt 0 ]; then + printf ' ⚠️ 有 %s 个 Agent 旧副本未能迁移(安装已完成,可稍后手动删除)\n' "$retire_failed" >&2 + fi if [ "$failed" -gt 0 ]; then printf ' ⚠️ 有 %s 个 Agent 目标安装 %s 失败\n' "$failed" "$SKILL_NAME" >&2 return 1 diff --git a/scripts/install-event.sh b/scripts/install-event.sh index 6ff57fc9..d4e65b6b 100755 --- a/scripts/install-event.sh +++ b/scripts/install-event.sh @@ -127,9 +127,44 @@ copy_tree() { return 1 } +# refresh_cache_tree +# Same staged, atomic swap as copy_tree, but for the ephemeral ~/.dws/skills +# caches: those trees are byte-reproducible from the release asset, so archiving +# every stale generation into ~/.dws/skill-backups only grows that directory +# without bound. The previous cache is deleted, exactly like install-devapp.sh — +# but only after the replacement is fully staged and published. +refresh_cache_tree() { + src="$1" + dest="$2" + parent="$(dirname "$dest")" + mkdir -p "$parent" || return 1 + stage="$(mktemp -d "$parent/.dws-cache.tmp.XXXXXX")" || return 1 + if ! cp -R "$src/." "$stage/"; then rm -rf "$stage"; return 1; fi + old="" + if [ -e "$dest" ] || [ -L "$dest" ]; then + old="$(mktemp -d "$parent/.dws-cache.old.XXXXXX")" || { rm -rf "$stage"; return 1; } + rmdir "$old" || { rm -rf "$stage" "$old"; return 1; } + if ! mv "$dest" "$old"; then rm -rf "$stage"; return 1; fi + fi + if mv "$stage" "$dest"; then + if [ -n "$old" ] && ! rm -rf "$old"; then + printf ' ⚠️ 新缓存已生效,但旧缓存清理失败: %s\n' "$old" >&2 + fi + return 0 + fi + rm -rf "$stage" + if [ -n "$old" ] && ! mv "$old" "$dest"; then + printf ' ⚠️ 缓存刷新失败,原缓存保留在 %s\n' "$old" >&2 + fi + return 1 +} + backup_skill_dir() { victim="$1" [ -e "$victim" ] || [ -L "$victim" ] || { printf '\n'; return 0; } + # Prune before creating this run's archive, so the backup the caller is about + # to depend on for rollback can never be a prune candidate. + prune_skill_backups stamp="$(date -u +%Y%m%d-%H%M%S)" name="$(printf '%s' "$victim" | sed 's#[/\\]#-#g; s#^-##')" backup_root="$HOME/.dws/skill-backups/$stamp" @@ -137,12 +172,43 @@ backup_skill_dir() { i=0 while [ -e "$backup" ] || [ -L "$backup" ]; do i=$((i + 1)); backup_root="$HOME/.dws/skill-backups/$stamp-$i"; backup="$backup_root/$name" + # Same bail-out as install-skills.sh: never spin forever on a pathological + # backup root, report it and keep the original directory instead. + if [ "$i" -gt 1000 ]; then + printf ' ⚠️ 备份目录冲突,保留原目录 %s\n' "$victim" >&2 + return 1 + fi done mkdir -p "$backup_root" || return 1 mv "$victim" "$backup" || return 1 printf '%s\n' "$backup" } +# prune_skill_backups keeps only the newest SKILL_BACKUP_KEEP stamped backup +# directories under $HOME/.dws/skill-backups, matching Go's skillBackupKeep / +# pruneSkillBackups. Glob order is lexicographic and the stamps are UTC +# YYYYmmdd-HHMMSS, so the oldest sort first. Best-effort: a removal failure is +# reported and never fatal. +SKILL_BACKUP_KEEP=5 +prune_skill_backups() { + prune_root="$HOME/.dws/skill-backups" + [ -d "$prune_root" ] || return 0 + prune_total=0 + for prune_entry in "$prune_root"/*; do + [ -d "$prune_entry" ] || continue + prune_total=$((prune_total + 1)) + done + [ "$prune_total" -gt "$SKILL_BACKUP_KEEP" ] || return 0 + prune_drop=$((prune_total - SKILL_BACKUP_KEEP)) + prune_seen=0 + for prune_entry in "$prune_root"/*; do + [ -d "$prune_entry" ] || continue + prune_seen=$((prune_seen + 1)) + [ "$prune_seen" -le "$prune_drop" ] || break + rm -rf "$prune_entry" || printf ' ⚠️ 旧 Skill 备份清理失败: %s\n' "$prune_entry" >&2 + done +} + same_physical_skill() { [ -d "$1" ] && [ -d "$2" ] || return 1 left="$(CDPATH= cd -- "$1" 2>/dev/null && pwd -P)" || return 1 @@ -221,14 +287,18 @@ install_skill_to_homes() { copy_tree "$src" "$canonical" || return 1 installed=1 failed=0 + retire_failed=0 for agent_dir in $(agent_skill_dirs); do base="$(resolve_agent_skill_base "$agent_dir")" agent_skill_base_detected "$agent_dir" "$base" || continue same_physical_skill "$base" "$HOME/.agents/skills" && continue if is_cleanup_only_agent_dir "$agent_dir"; then + # Cleanup-only: universal Agents read the canonical store directly, so + # nothing is installed here. A retire failure only leaves an obsolete copy + # behind and must never fail an otherwise complete install. if ! backup_skill_dir "$base/$skill_name" >/dev/null; then - printf ' ⚠️ Agent Skill 旧副本备份失败,保留原目录: %s\n' "$base/$skill_name" >&2 - failed=$((failed + 1)) + printf ' ⚠️ Agent Skill 旧副本备份失败,保留原目录(不影响本次安装): %s\n' "$base/$skill_name" >&2 + retire_failed=$((retire_failed + 1)) fi continue fi @@ -241,6 +311,9 @@ install_skill_to_homes() { fi installed=$((installed + 1)) done + if [ "$retire_failed" -gt 0 ]; then + printf ' ⚠️ 有 %s 个 Agent 旧副本未能迁移(安装已完成,可稍后手动删除)\n' "$retire_failed" >&2 + fi if [ "$failed" -gt 0 ]; then printf ' ⚠️ 有 %s 个 Agent 目标安装 %s 失败\n' "$failed" "$skill_name" >&2 return 1 @@ -298,12 +371,12 @@ install_skills_from_bundle() { event_cache="$HOME/.dws/skills/multi/$EVENT_SKILL_NAME" shared_cache="$HOME/.dws/skills/multi/$SHARED_SKILL_NAME" misc_cache="$HOME/.dws/skills/multi/$MISC_SKILL_NAME" - copy_tree "$event_src" "$event_cache" - copy_tree "$shared_src" "$shared_cache" - copy_tree "$misc_src" "$misc_cache" + refresh_cache_tree "$event_src" "$event_cache" || err "${EVENT_SKILL_NAME} 缓存刷新失败: ${event_cache}" + refresh_cache_tree "$shared_src" "$shared_cache" || err "${SHARED_SKILL_NAME} 缓存刷新失败: ${shared_cache}" + refresh_cache_tree "$misc_src" "$misc_cache" || err "${MISC_SKILL_NAME} 缓存刷新失败: ${misc_cache}" mono_cache="$HOME/.dws/skills/mono" - copy_tree "$mono_src" "$mono_cache" + refresh_cache_tree "$mono_src" "$mono_cache" || err "mono ${MONO_SKILL_NAME} 缓存刷新失败: ${mono_cache}" event_installed="$(install_skill_to_homes "$event_src" "$EVENT_SKILL_NAME")" \ || err "${EVENT_SKILL_NAME} 分发到 Agent 目录失败,详见上方告警" diff --git a/scripts/install-skills.sh b/scripts/install-skills.sh index c5b9a276..c3b3de3c 100755 --- a/scripts/install-skills.sh +++ b/scripts/install-skills.sh @@ -80,6 +80,31 @@ backup_and_remove_skill_dir() { return 1 } +# prune_skill_backups keeps only the newest DWS_SKILL_BACKUP_KEEP stamped backup +# directories under $HOME/.dws/skill-backups, matching Go's skillBackupKeep / +# pruneSkillBackups. Best-effort: a removal failure is reported, never fatal. +# Only safe to call once every backup manifest of this run has been consumed, +# because a single transaction can span more than one timestamp. +DWS_SKILL_BACKUP_KEEP=5 +prune_skill_backups() { + _psb_root="${HOME}/.dws/skill-backups" + [ -d "$_psb_root" ] || return 0 + _psb_total=0 + for _psb_entry in "$_psb_root"/*; do + [ -d "$_psb_entry" ] || continue + _psb_total=$((_psb_total + 1)) + done + [ "$_psb_total" -gt "$DWS_SKILL_BACKUP_KEEP" ] || return 0 + _psb_drop=$((_psb_total - DWS_SKILL_BACKUP_KEEP)) + _psb_seen=0 + for _psb_entry in "$_psb_root"/*; do + [ -d "$_psb_entry" ] || continue + _psb_seen=$((_psb_seen + 1)) + [ "$_psb_seen" -le "$_psb_drop" ] || break + rm -rf "$_psb_entry" || printf ' ⚠️ 旧 Skill 备份清理失败: %s\n' "$_psb_entry" + done +} + # A dingtalk-* prefix alone is not ownership evidence: market/user skills may # use it too. Ownership comes from the centralized skills-state.json. is_managed_multi_skill_dir() { @@ -475,42 +500,46 @@ multi_tree_has_skills() { return 1 } -# ~/.agents/skills is the canonical store under the universal convention. -# Universal Agents read it directly; other Agents receive -# relative links and fall back to copies only when links are unavailable. -is_universal_agent_dir() { - case "$1" in - ".config/agents/skills"|".gemini/antigravity/skills"|".gemini/antigravity-cli/skills"|".codex/skills"|".cursor/skills"|".deepagents/agent/skills"|".firebender/skills"|".gemini/skills"|".copilot/skills"|".config/opencode/skills"|".github/skills"|".windsurf/skills"|".cline/skills"|".amp/skills") return 0 ;; - *) return 1 ;; - esac -} - # Exact upstream registry (76 IDs): id|classification|effective-global-root. # `-` means no global directory; `.agents/skills` means canonical-direct. -readonly DWS_UPSTREAM_AGENT_REGISTRY='aider-desk|N|.aider-desk/skills amp|U|.config/agents/skills antigravity|U|.gemini/antigravity/skills antigravity-cli|U|.gemini/antigravity-cli/skills astrbot|N|.astrbot/data/skills autohand-code|N|.autohand/skills augment|N|.augment/skills bob|N|.bob/skills claude-code|N|.claude/skills openclaw|N|.openclaw/skills cline|U|.agents/skills codearts-agent|N|.codeartsdoer/skills codebuddy|N|.codebuddy/skills codemaker|N|.codemaker/skills codestudio|N|.codestudio/skills codex|U|.codex/skills command-code|N|.commandcode/skills continue|N|.continue/skills cortex|N|.snowflake/cortex/skills crush|N|.config/crush/skills cursor|U|.cursor/skills deepagents|U|.deepagents/agent/skills devin|N|.config/devin/skills dexto|U|.agents/skills droid|N|.factory/skills eve|N|- firebender|U|.firebender/skills forgecode|N|.forge/skills gemini-cli|U|.gemini/skills github-copilot|U|.copilot/skills goose|N|.config/goose/skills grok|N|.grok/skills hermes-agent|N|.hermes/skills inference-sh|N|.inferencesh/skills jazz|N|.jazz/skills junie|N|.junie/skills iflow-cli|N|.iflow/skills kilo|N|.kilocode/skills kimchi|N|.config/kimchi/harness/skills kimi-code-cli|U|.agents/skills kiro-cli|N|.kiro/skills kode|N|.kode/skills lingma|N|.lingma/skills loaf|U|.agents/skills mcpjam|N|.mcpjam/skills minimax-code|N|.minimax/skills mistral-vibe|N|.vibe/skills moxby|N|.moxby/skills mux|N|.mux/skills opencode|U|.config/opencode/skills openhands|N|.openhands/skills ona|N|.ona/skills pi|N|.pi/agent/skills qoder|N|.qoder/skills qoder-cn|N|.qoder-cn/skills qwen-code|N|.qwen/skills replit|U|.config/agents/skills reasonix|N|.reasonix/skills rovodev|N|.rovodev/skills roo|N|.roo/skills tabnine-cli|N|.tabnine/agent/skills terramind|N|.terramind/skills tinycloud|N|.tinycloud/skills trae|N|.trae/skills trae-cn|N|.trae-cn/skills warp|U|.agents/skills windsurf|N|.codeium/windsurf/skills zed|U|.agents/skills zcode|N|.zcode/skills zencoder|N|.zencoder/skills zenflow|N|.zencoder/skills neovate|N|.neovate/skills pochi|N|.pochi/skills promptscript|U|- adal|N|.adal/skills universal|U|.config/agents/skills' +# Not readonly: the library must stay re-sourceable inside one shell. +DWS_UPSTREAM_AGENT_REGISTRY='aider-desk|N|.aider-desk/skills amp|U|.config/agents/skills antigravity|U|.gemini/antigravity/skills antigravity-cli|U|.gemini/antigravity-cli/skills astrbot|N|.astrbot/data/skills autohand-code|N|.autohand/skills augment|N|.augment/skills bob|N|.bob/skills claude-code|N|.claude/skills openclaw|N|.openclaw/skills cline|U|.agents/skills codearts-agent|N|.codeartsdoer/skills codebuddy|N|.codebuddy/skills codemaker|N|.codemaker/skills codestudio|N|.codestudio/skills codex|U|.codex/skills command-code|N|.commandcode/skills continue|N|.continue/skills cortex|N|.snowflake/cortex/skills crush|N|.config/crush/skills cursor|U|.cursor/skills deepagents|U|.deepagents/agent/skills devin|N|.config/devin/skills dexto|U|.agents/skills droid|N|.factory/skills eve|N|- firebender|U|.firebender/skills forgecode|N|.forge/skills gemini-cli|U|.gemini/skills github-copilot|U|.copilot/skills goose|N|.config/goose/skills grok|N|.grok/skills hermes-agent|N|.hermes/skills inference-sh|N|.inferencesh/skills jazz|N|.jazz/skills junie|N|.junie/skills iflow-cli|N|.iflow/skills kilo|N|.kilocode/skills kimchi|N|.config/kimchi/harness/skills kimi-code-cli|U|.agents/skills kiro-cli|N|.kiro/skills kode|N|.kode/skills lingma|N|.lingma/skills loaf|U|.agents/skills mcpjam|N|.mcpjam/skills minimax-code|N|.minimax/skills mistral-vibe|N|.vibe/skills moxby|N|.moxby/skills mux|N|.mux/skills opencode|U|.config/opencode/skills openhands|N|.openhands/skills ona|N|.ona/skills pi|N|.pi/agent/skills qoder|N|.qoder/skills qoder-cn|N|.qoder-cn/skills qwen-code|N|.qwen/skills replit|U|.config/agents/skills reasonix|N|.reasonix/skills rovodev|N|.rovodev/skills roo|N|.roo/skills tabnine-cli|N|.tabnine/agent/skills terramind|N|.terramind/skills tinycloud|N|.tinycloud/skills trae|N|.trae/skills trae-cn|N|.trae-cn/skills warp|U|.agents/skills windsurf|N|.codeium/windsurf/skills zed|U|.agents/skills zcode|N|.zcode/skills zencoder|N|.zencoder/skills zenflow|N|.zencoder/skills neovate|N|.neovate/skills pochi|N|.pochi/skills promptscript|U|- adal|N|.adal/skills universal|U|.config/agents/skills' upstream_agent_registry() { for _uar_record in $DWS_UPSTREAM_AGENT_REGISTRY; do printf '%s\n' "$_uar_record"; done } +# DWS-only compatibility roots, same id|classification|root format. Qoderwork +# stays a real non-universal install target; the other four are global paths +# older DWS installers wrote by mistake, so they are retired like universal +# roots. Kept separate so the upstream pin above stays byte-comparable. +DWS_LEGACY_AGENT_ROOTS='dws-qoderwork|N|.qoderwork/skills dws-legacy-github|U|.github/skills dws-legacy-windsurf|U|.windsurf/skills dws-legacy-cline|U|.cline/skills dws-legacy-amp|U|.amp/skills' +legacy_agent_roots() { + for _lar_record in $DWS_LEGACY_AGENT_ROOTS; do printf '%s\n' "$_lar_record"; done +} + +# ~/.agents/skills is the canonical store under the universal convention. +# Universal Agents read it directly; other Agents receive relative links and +# fall back to copies only when links are unavailable. +# +# Both agent_skill_dirs and is_universal_agent_dir are DERIVED from the pinned +# registries above, so a registry edit changes real install behaviour instead of +# only a test count, and pin/behaviour drift is impossible by construction. +is_universal_agent_dir() { + for _iua_record in $DWS_UPSTREAM_AGENT_REGISTRY $DWS_LEGACY_AGENT_ROOTS; do + [ "${_iua_record##*|}" = "$1" ] || continue + case "$_iua_record" in + *"|U|"*) return 0 ;; + esac + done + return 1 +} + +# Effective global roots, in pinned registry order, de-duplicated. Records +# without a global root (`-`) and canonical-direct records (`.agents/skills`) +# are not per-Agent targets and are skipped. agent_skill_dirs() { - printf '%s\n' \ - ".config/agents/skills" ".gemini/antigravity/skills" ".gemini/antigravity-cli/skills" \ - ".codex/skills" ".cursor/skills" ".deepagents/agent/skills" ".firebender/skills" \ - ".gemini/skills" ".copilot/skills" ".config/opencode/skills" \ - ".aider-desk/skills" ".astrbot/data/skills" ".autohand/skills" ".augment/skills" \ - ".bob/skills" ".claude/skills" ".openclaw/skills" ".codeartsdoer/skills" \ - ".codebuddy/skills" ".codemaker/skills" ".codestudio/skills" ".commandcode/skills" \ - ".continue/skills" ".snowflake/cortex/skills" ".config/crush/skills" \ - ".config/devin/skills" ".factory/skills" ".forge/skills" ".config/goose/skills" \ - ".grok/skills" ".hermes/skills" ".inferencesh/skills" ".jazz/skills" ".junie/skills" \ - ".iflow/skills" ".kilocode/skills" ".config/kimchi/harness/skills" ".kiro/skills" \ - ".kode/skills" ".lingma/skills" ".mcpjam/skills" ".minimax/skills" ".vibe/skills" \ - ".moxby/skills" ".mux/skills" ".openhands/skills" ".ona/skills" ".pi/agent/skills" \ - ".qoder/skills" ".qoder-cn/skills" ".qwen/skills" ".reasonix/skills" \ - ".rovodev/skills" ".roo/skills" ".tabnine/agent/skills" ".terramind/skills" \ - ".tinycloud/skills" ".trae/skills" ".trae-cn/skills" ".codeium/windsurf/skills" \ - ".zcode/skills" ".zencoder/skills" ".neovate/skills" ".pochi/skills" ".adal/skills" \ - ".qoderwork/skills" ".github/skills" ".windsurf/skills" ".cline/skills" ".amp/skills" + { upstream_agent_registry; legacy_agent_roots; } | awk -F'|' ' + $3 != "-" && $3 != ".agents/skills" && !seen[$3]++ { print $3 }' } resolve_agent_skill_base() { @@ -548,8 +577,11 @@ agent_skill_base_detected() { same_physical_skill_root() { [ -d "$1" ] && [ -d "$2" ] || return 1 - _sps_left="$(cd -P "$1" 2>/dev/null && pwd)" || return 1 - _sps_right="$(cd -P "$2" 2>/dev/null && pwd)" || return 1 + # CDPATH= and -- are required: an exported CDPATH makes `cd` echo the resolved + # directory into the command substitution, which would silently report two + # identical roots as different. + _sps_left="$(CDPATH= cd -- "$1" 2>/dev/null && pwd -P)" || return 1 + _sps_right="$(CDPATH= cd -- "$2" 2>/dev/null && pwd -P)" || return 1 [ "$_sps_left" = "$_sps_right" ] } @@ -573,26 +605,55 @@ retire_agent_skill_root() { rm -rf "$_rgs_stage" } +# link_canonical_skills_to_base [bundle-src] +# is the installed multi bundle and is REQUIRED in multi mode: the +# link set must come from the bundle, never from the whole canonical store. +# ~/.agents/skills is now shared, so third-party/user Skills live there too and +# must not be published into every detected Agent root (they are absent from +# skills-state.json, so nothing could ever reclaim those links). Go +# publishLinkedUpgradeTarget and build/npm/install.js pass the bundle skill list +# the same way. link_canonical_skills_to_base() { - _lcs_root="$1"; _lcs_base="$2"; _lcs_mode="$3" + _lcs_root="$1"; _lcs_base="$2"; _lcs_mode="$3"; _lcs_bundle="${4-}" _lcs_canonical="$_lcs_root/.agents/skills" mkdir -p "$_lcs_base" || return 1 same_physical_skill_root "$_lcs_base" "$_lcs_canonical" && return 0 + if [ "$_lcs_mode" = "mono" ]; then + _lcs_names="dws" + else + [ -n "$_lcs_bundle" ] && [ -d "$_lcs_bundle" ] || return 1 + _lcs_names="" + for _lcs_skill in "$_lcs_bundle"/*/; do + [ -f "${_lcs_skill}SKILL.md" ] || continue + _lcs_names="$_lcs_names $(basename "$_lcs_skill")" + done + [ -n "$_lcs_names" ] || return 1 + fi + # Collision pre-flight: the victim loop below deliberately refuses to back up + # entries that are not DWS-managed, and publication must never replace them. + # Report the exact colliding paths before touching anything, so the fall back + # to the legacy full-copy layout is observable and actionable. + _lcs_collision=0 + for _lcs_name in $_lcs_names; do + _lcs_dest="$_lcs_base/$_lcs_name" + { [ -e "$_lcs_dest" ] || [ -L "$_lcs_dest" ]; } || continue + same_physical_skill_root "$_lcs_dest" "$_lcs_canonical/$_lcs_name" && continue + [ "$_lcs_name" = "dws" ] && continue + is_managed_multi_skill_dir "$_lcs_dest" && continue + printf ' ⚠️ %s 已存在且不是 DWS 安装的 Skill,无法在此建立指向 %s 的共享链接\n' "$_lcs_dest" "$_lcs_canonical/$_lcs_name" + _lcs_collision=1 + done + if [ "$_lcs_collision" -eq 1 ]; then + printf ' ⚠️ 已保留上述目录(不会删除用户数据),该 Agent 改用独立副本方式安装;\n' + printf ' 移走或改名后重新运行安装即可切换为共享 %s 布局。\n' "$_lcs_canonical" + return 1 + fi _lcs_base_real="$(CDPATH= cd -- "$_lcs_base" && pwd -P)" || return 1 _lcs_stage="$(mktemp -d "$_lcs_base/.dws-link-set.XXXXXX")" || return 1 _lcs_backups="$_lcs_stage/.backups"; _lcs_published="$_lcs_stage/.published" _lcs_stage_token="$(basename "$_lcs_stage")" : > "$_lcs_backups" || { rm -rf "$_lcs_stage"; return 1; } : > "$_lcs_published" || { rm -rf "$_lcs_stage"; return 1; } - if [ "$_lcs_mode" = "mono" ]; then - _lcs_names="dws" - else - _lcs_names="" - for _lcs_skill in "$_lcs_canonical"/*/; do - [ -f "${_lcs_skill}SKILL.md" ] || continue - _lcs_names="$_lcs_names $(basename "$_lcs_skill")" - done - fi _lcs_publish_names="" for _lcs_name in $_lcs_names; do if same_physical_skill_root "$_lcs_base/$_lcs_name" "$_lcs_canonical/$_lcs_name"; then continue; fi @@ -619,17 +680,24 @@ link_canonical_skills_to_base() { _lcs_link_target="$(readlink "$_lcs_staged" 2>/dev/null)" || { restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1 } - # Keep the staged hardlink alive until the whole transaction finishes. It - # pins the inode, so a concurrently recreated link cannot reuse the - # identity recorded in the publication manifest. + # The staged symlink keeps its inode across the rename below (the staging + # directory is a sibling inside the same Agent root), so the identity + # recorded in the publication manifest stays exact. _lcs_inode="$(skill_link_inode "$_lcs_staged")" || { restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1 } - # Hard-link the staged symlink itself. This never overwrites a - # concurrently-created destination. If that destination became a - # directory, the unique staged basename can only be linked inside it; - # post-validation detects that case and removes only our link. - if ! ln -P "$_lcs_staged" "$_lcs_dest" 2>/dev/null || ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then + # Anything still present here appeared after the pre-flight scan, i.e. a + # concurrent writer. Never let `mv` clobber it. + if [ -e "$_lcs_dest" ] || [ -L "$_lcs_dest" ]; then + restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1 + fi + # Publish by moving the staged symlink. `mv` is POSIX; `ln -P` is not (it is + # absent from BusyBox `ln`, which silently degraded every non-universal + # Agent to the copy layout on Alpine and most containers). install-event.sh + # and install-devapp.sh already publish this way. The staged basename stays + # unique, so a destination a concurrent process turned into a directory can + # only receive our link and post-validation removes exactly that. + if ! mv "$_lcs_staged" "$_lcs_dest" 2>/dev/null || ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then cleanup_nested_staged_link "$_lcs_dest" "$_lcs_stage_name" "$_lcs_link_target" "$_lcs_inode" || true restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true; rm -rf "$_lcs_stage"; return 1 fi @@ -649,6 +717,7 @@ install_multi_skills_to_root() { installed=0 attempted=1 failed=0 + retire_failed=0 if _install_multi_to_base "$multi_src" "$root/.agents/skills" "$root" ".agents/skills"; then installed=1; else failed=1; fi [ "$installed" -gt 0 ] || { printf ' ⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败\n'; return 1; } for agent_dir in $(agent_skill_dirs) @@ -658,10 +727,10 @@ install_multi_skills_to_root() { same_physical_skill_root "$base_dir" "$root/.agents/skills" && continue attempted=$((attempted + 1)) if is_universal_agent_dir "$agent_dir"; then - retire_agent_skill_root "$root" "$base_dir" || failed=$((failed + 1)) + retire_agent_skill_root "$root" "$base_dir" || retire_failed=$((retire_failed + 1)) continue fi - if link_canonical_skills_to_base "$root" "$base_dir" multi; then + if link_canonical_skills_to_base "$root" "$base_dir" multi "$multi_src"; then installed=$((installed + 1)) else if _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir"; then @@ -676,6 +745,9 @@ install_multi_skills_to_root() { printf ' ⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败\n' return 1 fi + if [ "$retire_failed" -gt 0 ]; then + printf ' ⚠️ 有 %s 个 Agent 旧副本未能迁移(安装已完成,可稍后手动删除)\n' "$retire_failed" + fi if [ "$failed" -gt 0 ]; then printf ' ⚠️ 有 %s 个 Agent 目标安装失败\n' "$failed" return 1 @@ -841,6 +913,7 @@ install_skills_to_root() { installed=0 attempted=1 failed=0 + retire_failed=0 if _install_mono_to_base "$skill_src" "$root/.agents/skills" "$root/.agents/skills/$SKILL_NAME"; then installed=1; else failed=1; fi [ "$installed" -gt 0 ] || { printf ' ⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败\n'; return 1; } for agent_dir in $(agent_skill_dirs) @@ -850,7 +923,7 @@ install_skills_to_root() { same_physical_skill_root "$base_dir" "$root/.agents/skills" && continue attempted=$((attempted + 1)) if is_universal_agent_dir "$agent_dir"; then - retire_agent_skill_root "$root" "$base_dir" || failed=$((failed + 1)) + retire_agent_skill_root "$root" "$base_dir" || retire_failed=$((retire_failed + 1)) continue fi if link_canonical_skills_to_base "$root" "$base_dir" mono; then @@ -868,6 +941,9 @@ install_skills_to_root() { printf ' ⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败\n' return 1 fi + if [ "$retire_failed" -gt 0 ]; then + printf ' ⚠️ 有 %s 个 Agent 旧副本未能迁移(安装已完成,可稍后手动删除)\n' "$retire_failed" + fi if [ "$failed" -gt 0 ]; then printf ' ⚠️ 有 %s 个 Agent 目标安装 mono Skill 失败\n' "$failed" return 1 @@ -949,6 +1025,10 @@ main() { fi fi + # Every transaction of this run has finished, so old stamped archives can no + # longer be needed for a rollback. + prune_skill_backups + printf '\n' printf ' 📖 Skill includes:\n' printf ' • SKILL.md — Main skill with product overview and intent routing\n' diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 4cda9a7d..99712993 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -704,6 +704,57 @@ function Publish-SkillCache { } } +# Get-SkillBackupName encodes the HOME-relative path of a backed-up Skill +# directory ('.codex\skills\dingtalk-chat' → '.codex-skills-dingtalk-chat') so +# copies retired from different Agent roots stay distinguishable inside one +# stamp. Mirrors build/npm/install.js and internal/upgrade/paths.go. Paths +# outside HOME fall back to the bare leaf. +function Get-SkillBackupName { + param([string]$Dir) + $leaf = Split-Path $Dir -Leaf + try { + $full = [System.IO.Path]::GetFullPath($Dir) + $root = [System.IO.Path]::GetFullPath($HOME).TrimEnd([char[]]@('\', '/')) + } catch { + return $leaf + } + if ([string]::IsNullOrWhiteSpace($root)) { return $leaf } + foreach ($sep in @('\', '/')) { + $prefix = $root + $sep + if ($full.Length -gt $prefix.Length -and + $full.StartsWith($prefix, [System.StringComparison]::OrdinalIgnoreCase)) { + $rel = $full.Substring($prefix.Length).Trim([char[]]@('\', '/')) + if (![string]::IsNullOrWhiteSpace($rel)) { return ($rel -replace '[\\/]+', '-') } + } + } + return $leaf +} + +# SkillBackupKeep bounds $HOME\.dws\skill-backups growth: only the newest +# stamped backup directories are kept, matching skillBackupKeep and +# pruneSkillBackups in internal/upgrade/paths.go. +$SkillBackupKeep = 5 +$script:SkillBackupRootsThisRun = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) + +# Remove-OldSkillBackups deletes the oldest stamped backup directories once +# more than $SkillBackupKeep remain. Stamps sort lexicographically in +# chronological order, so name order is age order. Roots created during this +# run are never pruned: an in-flight transaction still needs them to roll +# back. Best-effort — a prune failure never fails the install. +function Remove-OldSkillBackups { + $root = Join-Path $HOME ".dws\skill-backups" + if (!(Test-Path -LiteralPath $root -PathType Container)) { return } + $dirs = @(Get-ChildItem -LiteralPath $root -Directory -Force -ErrorAction SilentlyContinue | + Sort-Object -Property Name) + $excess = $dirs.Count - $SkillBackupKeep + foreach ($dir in $dirs) { + if ($excess -le 0) { break } + if ($script:SkillBackupRootsThisRun.Contains($dir.FullName)) { continue } + Remove-Item -LiteralPath $dir.FullName -Recurse -Force -ErrorAction SilentlyContinue + $excess-- + } +} + # Backup-SkillDir moves $Dir into $HOME\.dws\skill-backups\\ # instead of destroying it (non-interactive installs cannot confirm, so # removals must stay reversible). Missing paths are a no-op success. On any @@ -718,11 +769,13 @@ function Backup-SkillDir { if (!(Test-SkillPathLexically -Path $Dir)) { return $true } $backupRoot = Join-Path $HOME ".dws\skill-backups" $stamp = [DateTime]::UtcNow.ToString("yyyyMMdd-HHmmss") - $name = Split-Path $Dir -Leaf - $target = Join-Path (Join-Path $backupRoot $stamp) $name + $name = Get-SkillBackupName -Dir $Dir + $targetRoot = Join-Path $backupRoot $stamp + $target = Join-Path $targetRoot $name $i = 1 while (Test-SkillPathLexically -Path $target) { - $target = Join-Path (Join-Path $backupRoot "$stamp-$i") $name + $targetRoot = Join-Path $backupRoot "$stamp-$i" + $target = Join-Path $targetRoot $name $i++ if ($i -gt 1000) { Write-Say "⚠️ 备份目录冲突,保留原目录 $Dir" @@ -737,6 +790,12 @@ function Backup-SkillDir { return $false } if ($null -ne $BackupPath) { $BackupPath.Value = $target } + try { + $script:SkillBackupRootsThisRun.Add([System.IO.Path]::GetFullPath($targetRoot)) | Out-Null + Remove-OldSkillBackups + } catch { + Write-Say "⚠️ 旧备份清理失败(备份本身已成功): $_" + } Write-Say " × 已备份并移除 $Dir → $target" return $true } @@ -942,7 +1001,7 @@ function Move-AgentSkillRootToBackup { } function Publish-CanonicalSkillLinks { - param([string]$Root, [string]$BaseDir, [string]$Mode) + param([string]$Root, [string]$BaseDir, [string]$Mode, [string[]]$BundleNames = @()) $canonical = Join-Path $Root ".agents\skills" if (!(Test-Path $BaseDir)) { New-Item -ItemType Directory -Path $BaseDir -Force | Out-Null } if (Test-SamePhysicalSkillRoot -Left $BaseDir -Right $canonical) { return $true } @@ -953,10 +1012,17 @@ function Publish-CanonicalSkillLinks { New-Item -ItemType Directory -Path $stageRoot -Force -ErrorAction Stop | Out-Null if ($Mode -eq "mono") { $names = @($SkillName) + } elseif ($BundleNames.Count -gt 0) { + # The canonical store is SHARED: enumerate the link set from the + # installed bundle, never from ~\.agents\skills, or third-party and + # user skills would be republished into every Agent root. + $names = @($BundleNames) } else { - $names = @(Get-ChildItem -Path $canonical -Directory -ErrorAction Stop | Where-Object { - Test-Path (Join-Path $_.FullName "SKILL.md") - } | ForEach-Object { $_.Name }) + # No bundle list means we cannot tell DWS skills apart from the + # user's own entries in the shared store. Fail this Agent (the + # caller degrades to a copy install) instead of guessing, matching + # link_canonical_skills_to_base in scripts/install-skills.sh. + throw "multi mode requires the installed bundle skill names" } $publishNames = @() foreach ($name in $names) { @@ -971,6 +1037,12 @@ function Publish-CanonicalSkillLinks { if ($existing.FullName -eq $stageRoot) { continue } if (Test-ManagedMultiSkillDir -Dir $existing.FullName) { $victims.Add($existing.FullName) } } + # Every published name replaces whatever occupies its destination, even + # when that copy predates central ownership metadata; the loop below + # still skips destinations that are already correct links. + foreach ($name in $names) { + $victims.Add((Join-Path $BaseDir $name)) + } $seen = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) foreach ($victim in $victims) { if (!$seen.Add($victim)) { continue } @@ -1315,7 +1387,13 @@ function Install-SkillsToHomes { if (Test-SamePhysicalSkillRoot -Left $baseDir -Right $canonical) { continue } $attempted++ if ($agent.Universal) { - if (!(Move-AgentSkillRootToBackup -Root $Root -BaseDir $baseDir)) { $failed++ } + # Cleanup-only migration: universal Agents read the canonical store + # directly, so nothing is installed here. Leaving an obsolete copy + # behind is a warning, never a reason to fail an install whose + # canonical store and links all succeeded. + if (!(Move-AgentSkillRootToBackup -Root $Root -BaseDir $baseDir)) { + Write-Say "⚠️ Agent Skill 旧副本迁移失败(不影响本次安装): $baseDir" + } continue } if (Publish-CanonicalSkillLinks -Root $Root -BaseDir $baseDir -Mode "mono") { @@ -1370,6 +1448,11 @@ function Install-MultiSkillsToHomes { $attempted = 1 $failed = 0 $canonical = Join-Path $Root ".agents\skills" + # The link set must come from the installed bundle, not from the shared + # canonical store (which may also hold user/third-party skills). + $bundleNames = @(Get-ChildItem -Path $MultiSrc -Directory -ErrorAction SilentlyContinue | Where-Object { + Test-Path (Join-Path $_.FullName "SKILL.md") + } | ForEach-Object { $_.Name }) if (Install-MultiToBase -MultiSrc $MultiSrc -BaseDir $canonical -Root $Root -AgentDir ".agents\skills") { $installed++ } else { $failed++ } if ($installed -eq 0) { return $false } $seenBases = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) @@ -1382,10 +1465,14 @@ function Install-MultiSkillsToHomes { if (Test-SamePhysicalSkillRoot -Left $baseDir -Right $canonical) { continue } $attempted++ if ($agent.Universal) { - if (!(Move-AgentSkillRootToBackup -Root $Root -BaseDir $baseDir)) { $failed++ } + # Cleanup-only migration (see Install-SkillsToHomes): a retire + # failure must not fail an otherwise complete install. + if (!(Move-AgentSkillRootToBackup -Root $Root -BaseDir $baseDir)) { + Write-Say "⚠️ Agent Skill 旧副本迁移失败(不影响本次安装): $baseDir" + } continue } - if (Publish-CanonicalSkillLinks -Root $Root -BaseDir $baseDir -Mode "multi") { + if (Publish-CanonicalSkillLinks -Root $Root -BaseDir $baseDir -Mode "multi" -BundleNames $bundleNames) { $installed++ } else { if (Install-MultiToBase -MultiSrc $MultiSrc -BaseDir $baseDir -Root $Root -AgentDir $agent.Dir) { diff --git a/scripts/install.sh b/scripts/install.sh index 45e5a688..a9aa5883 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -98,6 +98,31 @@ backup_and_remove_skill_dir() { return 1 } +# prune_skill_backups keeps only the newest DWS_SKILL_BACKUP_KEEP stamped backup +# directories under $HOME/.dws/skill-backups, matching Go's skillBackupKeep / +# pruneSkillBackups. Best-effort: a removal failure is reported, never fatal. +# Only safe to call once every backup manifest of this run has been consumed, +# because a single transaction can span more than one timestamp. +DWS_SKILL_BACKUP_KEEP=5 +prune_skill_backups() { + _psb_root="${HOME}/.dws/skill-backups" + [ -d "$_psb_root" ] || return 0 + _psb_total=0 + for _psb_entry in "$_psb_root"/*; do + [ -d "$_psb_entry" ] || continue + _psb_total=$((_psb_total + 1)) + done + [ "$_psb_total" -gt "$DWS_SKILL_BACKUP_KEEP" ] || return 0 + _psb_drop=$((_psb_total - DWS_SKILL_BACKUP_KEEP)) + _psb_seen=0 + for _psb_entry in "$_psb_root"/*; do + [ -d "$_psb_entry" ] || continue + _psb_seen=$((_psb_seen + 1)) + [ "$_psb_seen" -le "$_psb_drop" ] || break + rm -rf "$_psb_entry" || say " ⚠️ 旧 Skill 备份清理失败: $_psb_entry" + done +} + # A dingtalk-* prefix alone is not ownership evidence: market/user skills may # use it too. Ownership comes from the centralized skills-state.json. is_managed_multi_skill_dir() { @@ -704,46 +729,47 @@ _install_mono_to_base() { say "✅ Skills → ${_mono_label} (${_mono_count} files)" } -# ~/.agents/skills is the canonical store under the universal convention. -# Universal Agents read it directly; other Agents receive -# relative links and fall back to copies only when links are unavailable. -is_universal_agent_dir() { - case "$1" in - ".config/agents/skills"|".gemini/antigravity/skills"|".gemini/antigravity-cli/skills"|".codex/skills"|".cursor/skills"|".deepagents/agent/skills"|".firebender/skills"|".gemini/skills"|".copilot/skills"|".config/opencode/skills"|".github/skills"|".windsurf/skills"|".cline/skills"|".amp/skills") return 0 ;; - *) return 1 ;; - esac -} - # Exact upstream registry (76 IDs): id|classification|effective-global-root. # `-` means no global directory; `.agents/skills` means canonical-direct. -readonly DWS_UPSTREAM_AGENT_REGISTRY='aider-desk|N|.aider-desk/skills amp|U|.config/agents/skills antigravity|U|.gemini/antigravity/skills antigravity-cli|U|.gemini/antigravity-cli/skills astrbot|N|.astrbot/data/skills autohand-code|N|.autohand/skills augment|N|.augment/skills bob|N|.bob/skills claude-code|N|.claude/skills openclaw|N|.openclaw/skills cline|U|.agents/skills codearts-agent|N|.codeartsdoer/skills codebuddy|N|.codebuddy/skills codemaker|N|.codemaker/skills codestudio|N|.codestudio/skills codex|U|.codex/skills command-code|N|.commandcode/skills continue|N|.continue/skills cortex|N|.snowflake/cortex/skills crush|N|.config/crush/skills cursor|U|.cursor/skills deepagents|U|.deepagents/agent/skills devin|N|.config/devin/skills dexto|U|.agents/skills droid|N|.factory/skills eve|N|- firebender|U|.firebender/skills forgecode|N|.forge/skills gemini-cli|U|.gemini/skills github-copilot|U|.copilot/skills goose|N|.config/goose/skills grok|N|.grok/skills hermes-agent|N|.hermes/skills inference-sh|N|.inferencesh/skills jazz|N|.jazz/skills junie|N|.junie/skills iflow-cli|N|.iflow/skills kilo|N|.kilocode/skills kimchi|N|.config/kimchi/harness/skills kimi-code-cli|U|.agents/skills kiro-cli|N|.kiro/skills kode|N|.kode/skills lingma|N|.lingma/skills loaf|U|.agents/skills mcpjam|N|.mcpjam/skills minimax-code|N|.minimax/skills mistral-vibe|N|.vibe/skills moxby|N|.moxby/skills mux|N|.mux/skills opencode|U|.config/opencode/skills openhands|N|.openhands/skills ona|N|.ona/skills pi|N|.pi/agent/skills qoder|N|.qoder/skills qoder-cn|N|.qoder-cn/skills qwen-code|N|.qwen/skills replit|U|.config/agents/skills reasonix|N|.reasonix/skills rovodev|N|.rovodev/skills roo|N|.roo/skills tabnine-cli|N|.tabnine/agent/skills terramind|N|.terramind/skills tinycloud|N|.tinycloud/skills trae|N|.trae/skills trae-cn|N|.trae-cn/skills warp|U|.agents/skills windsurf|N|.codeium/windsurf/skills zed|U|.agents/skills zcode|N|.zcode/skills zencoder|N|.zencoder/skills zenflow|N|.zencoder/skills neovate|N|.neovate/skills pochi|N|.pochi/skills promptscript|U|- adal|N|.adal/skills universal|U|.config/agents/skills' +# Not readonly: the library must stay re-sourceable inside one shell. +DWS_UPSTREAM_AGENT_REGISTRY='aider-desk|N|.aider-desk/skills amp|U|.config/agents/skills antigravity|U|.gemini/antigravity/skills antigravity-cli|U|.gemini/antigravity-cli/skills astrbot|N|.astrbot/data/skills autohand-code|N|.autohand/skills augment|N|.augment/skills bob|N|.bob/skills claude-code|N|.claude/skills openclaw|N|.openclaw/skills cline|U|.agents/skills codearts-agent|N|.codeartsdoer/skills codebuddy|N|.codebuddy/skills codemaker|N|.codemaker/skills codestudio|N|.codestudio/skills codex|U|.codex/skills command-code|N|.commandcode/skills continue|N|.continue/skills cortex|N|.snowflake/cortex/skills crush|N|.config/crush/skills cursor|U|.cursor/skills deepagents|U|.deepagents/agent/skills devin|N|.config/devin/skills dexto|U|.agents/skills droid|N|.factory/skills eve|N|- firebender|U|.firebender/skills forgecode|N|.forge/skills gemini-cli|U|.gemini/skills github-copilot|U|.copilot/skills goose|N|.config/goose/skills grok|N|.grok/skills hermes-agent|N|.hermes/skills inference-sh|N|.inferencesh/skills jazz|N|.jazz/skills junie|N|.junie/skills iflow-cli|N|.iflow/skills kilo|N|.kilocode/skills kimchi|N|.config/kimchi/harness/skills kimi-code-cli|U|.agents/skills kiro-cli|N|.kiro/skills kode|N|.kode/skills lingma|N|.lingma/skills loaf|U|.agents/skills mcpjam|N|.mcpjam/skills minimax-code|N|.minimax/skills mistral-vibe|N|.vibe/skills moxby|N|.moxby/skills mux|N|.mux/skills opencode|U|.config/opencode/skills openhands|N|.openhands/skills ona|N|.ona/skills pi|N|.pi/agent/skills qoder|N|.qoder/skills qoder-cn|N|.qoder-cn/skills qwen-code|N|.qwen/skills replit|U|.config/agents/skills reasonix|N|.reasonix/skills rovodev|N|.rovodev/skills roo|N|.roo/skills tabnine-cli|N|.tabnine/agent/skills terramind|N|.terramind/skills tinycloud|N|.tinycloud/skills trae|N|.trae/skills trae-cn|N|.trae-cn/skills warp|U|.agents/skills windsurf|N|.codeium/windsurf/skills zed|U|.agents/skills zcode|N|.zcode/skills zencoder|N|.zencoder/skills zenflow|N|.zencoder/skills neovate|N|.neovate/skills pochi|N|.pochi/skills promptscript|U|- adal|N|.adal/skills universal|U|.config/agents/skills' upstream_agent_registry() { for _uar_record in $DWS_UPSTREAM_AGENT_REGISTRY; do printf '%s\n' "$_uar_record"; done } -# Effective global roots from vercel-labs/skills. Duplicate upstream roots -# (amp/replit/universal and zencoder/zenflow) deliberately appear once. -# Qoderwork is a DWS-only non-universal target. The final four entries are -# cleanup-only compatibility paths from older DWS installers. +# DWS-only compatibility roots, same id|classification|root format. Qoderwork +# stays a real non-universal install target; the other four are global paths +# older DWS installers wrote by mistake, so they are retired like universal +# roots. Kept separate so the upstream pin above stays byte-comparable. +DWS_LEGACY_AGENT_ROOTS='dws-qoderwork|N|.qoderwork/skills dws-legacy-github|U|.github/skills dws-legacy-windsurf|U|.windsurf/skills dws-legacy-cline|U|.cline/skills dws-legacy-amp|U|.amp/skills' +legacy_agent_roots() { + for _lar_record in $DWS_LEGACY_AGENT_ROOTS; do printf '%s\n' "$_lar_record"; done +} + +# ~/.agents/skills is the canonical store under the universal convention. +# Universal Agents read it directly; other Agents receive relative links and +# fall back to copies only when links are unavailable. +# +# Both agent_skill_dirs and is_universal_agent_dir are DERIVED from the pinned +# registries above, so a registry edit changes real install behaviour instead of +# only a test count, and pin/behaviour drift is impossible by construction. +is_universal_agent_dir() { + for _iua_record in $DWS_UPSTREAM_AGENT_REGISTRY $DWS_LEGACY_AGENT_ROOTS; do + [ "${_iua_record##*|}" = "$1" ] || continue + case "$_iua_record" in + *"|U|"*) return 0 ;; + esac + done + return 1 +} + +# Effective global roots from vercel-labs/skills, in pinned registry order and +# de-duplicated (amp/replit/universal and zencoder/zenflow share a root). +# Records without a global root (`-`) and canonical-direct records +# (`.agents/skills`) are not per-Agent targets and are skipped. agent_skill_dirs() { - printf '%s\n' \ - ".config/agents/skills" ".gemini/antigravity/skills" ".gemini/antigravity-cli/skills" \ - ".codex/skills" ".cursor/skills" ".deepagents/agent/skills" ".firebender/skills" \ - ".gemini/skills" ".copilot/skills" ".config/opencode/skills" \ - ".aider-desk/skills" ".astrbot/data/skills" ".autohand/skills" ".augment/skills" \ - ".bob/skills" ".claude/skills" ".openclaw/skills" ".codeartsdoer/skills" \ - ".codebuddy/skills" ".codemaker/skills" ".codestudio/skills" ".commandcode/skills" \ - ".continue/skills" ".snowflake/cortex/skills" ".config/crush/skills" \ - ".config/devin/skills" ".factory/skills" ".forge/skills" ".config/goose/skills" \ - ".grok/skills" ".hermes/skills" ".inferencesh/skills" ".jazz/skills" ".junie/skills" \ - ".iflow/skills" ".kilocode/skills" ".config/kimchi/harness/skills" ".kiro/skills" \ - ".kode/skills" ".lingma/skills" ".mcpjam/skills" ".minimax/skills" ".vibe/skills" \ - ".moxby/skills" ".mux/skills" ".openhands/skills" ".ona/skills" ".pi/agent/skills" \ - ".qoder/skills" ".qoder-cn/skills" ".qwen/skills" ".reasonix/skills" \ - ".rovodev/skills" ".roo/skills" ".tabnine/agent/skills" ".terramind/skills" \ - ".tinycloud/skills" ".trae/skills" ".trae-cn/skills" ".codeium/windsurf/skills" \ - ".zcode/skills" ".zencoder/skills" ".neovate/skills" ".pochi/skills" ".adal/skills" \ - ".qoderwork/skills" ".github/skills" ".windsurf/skills" ".cline/skills" ".amp/skills" + { upstream_agent_registry; legacy_agent_roots; } | awk -F'|' ' + $3 != "-" && $3 != ".agents/skills" && !seen[$3]++ { print $3 }' } resolve_agent_skill_base() { @@ -781,8 +807,11 @@ agent_skill_base_detected() { same_physical_skill_root() { [ -d "$1" ] && [ -d "$2" ] || return 1 - _sps_left="$(cd -P "$1" 2>/dev/null && pwd)" || return 1 - _sps_right="$(cd -P "$2" 2>/dev/null && pwd)" || return 1 + # CDPATH= and -- are required: an exported CDPATH makes `cd` echo the resolved + # directory into the command substitution, which would silently report two + # identical roots as different. + _sps_left="$(CDPATH= cd -- "$1" 2>/dev/null && pwd -P)" || return 1 + _sps_right="$(CDPATH= cd -- "$2" 2>/dev/null && pwd -P)" || return 1 [ "$_sps_left" = "$_sps_right" ] } @@ -806,13 +835,52 @@ retire_agent_skill_root() { rm -rf "$_rgs_stage" } +# link_canonical_skills_to_base [bundle-src] +# is the installed multi bundle and is REQUIRED in multi mode: the +# link set must come from the bundle, never from the whole canonical store. +# ~/.agents/skills is now shared, so third-party/user Skills live there too and +# must not be published into every detected Agent root (they are absent from +# skills-state.json, so nothing could ever reclaim those links). Go +# publishLinkedUpgradeTarget and build/npm/install.js pass the bundle skill list +# the same way. link_canonical_skills_to_base() { _lcs_root="$1" _lcs_base="$2" _lcs_mode="$3" + _lcs_bundle="${4-}" _lcs_canonical="$_lcs_root/.agents/skills" mkdir -p "$_lcs_base" || return 1 same_physical_skill_root "$_lcs_base" "$_lcs_canonical" && return 0 + if [ "$_lcs_mode" = "mono" ]; then + _lcs_names="dws" + else + [ -n "$_lcs_bundle" ] && [ -d "$_lcs_bundle" ] || return 1 + _lcs_names="" + for _lcs_skill in "$_lcs_bundle"/*/; do + [ -f "${_lcs_skill}SKILL.md" ] || continue + _lcs_names="$_lcs_names $(basename "$_lcs_skill")" + done + [ -n "$_lcs_names" ] || return 1 + fi + # Collision pre-flight: the victim loop below deliberately refuses to back up + # entries that are not DWS-managed, and publication must never replace them. + # Report the exact colliding paths before touching anything, so the fall back + # to the legacy full-copy layout is observable and actionable. + _lcs_collision=0 + for _lcs_name in $_lcs_names; do + _lcs_dest="$_lcs_base/$_lcs_name" + { [ -e "$_lcs_dest" ] || [ -L "$_lcs_dest" ]; } || continue + same_physical_skill_root "$_lcs_dest" "$_lcs_canonical/$_lcs_name" && continue + [ "$_lcs_name" = "dws" ] && continue + is_managed_multi_skill_dir "$_lcs_dest" && continue + say " ⚠️ $_lcs_dest 已存在且不是 DWS 安装的 Skill,无法在此建立指向 $_lcs_canonical/$_lcs_name 的共享链接" + _lcs_collision=1 + done + if [ "$_lcs_collision" -eq 1 ]; then + say " ⚠️ 已保留上述目录(不会删除用户数据),该 Agent 改用独立副本方式安装;" + say " 移走或改名后重新运行安装即可切换为共享 $_lcs_canonical 布局。" + return 1 + fi _lcs_base_real="$(CDPATH= cd -- "$_lcs_base" && pwd -P)" || return 1 _lcs_stage="$(mktemp -d "$_lcs_base/.dws-link-set.XXXXXX")" || return 1 _lcs_backups="$_lcs_stage/.backups" @@ -820,15 +888,6 @@ link_canonical_skills_to_base() { _lcs_stage_token="$(basename "$_lcs_stage")" : > "$_lcs_backups" || { rm -rf "$_lcs_stage"; return 1; } : > "$_lcs_published" || { rm -rf "$_lcs_stage"; return 1; } - if [ "$_lcs_mode" = "mono" ]; then - _lcs_names="dws" - else - _lcs_names="" - for _lcs_skill in "$_lcs_canonical"/*/; do - [ -f "${_lcs_skill}SKILL.md" ] || continue - _lcs_names="$_lcs_names $(basename "$_lcs_skill")" - done - fi _lcs_publish_names="" for _lcs_name in $_lcs_names; do if same_physical_skill_root "$_lcs_base/$_lcs_name" "$_lcs_canonical/$_lcs_name"; then @@ -862,19 +921,28 @@ link_canonical_skills_to_base() { rm -rf "$_lcs_stage" return 1 } - # Keep the staged hardlink alive until the whole transaction finishes. It - # pins the inode, so a concurrently recreated link cannot reuse the - # identity recorded in the publication manifest. + # The staged symlink keeps its inode across the rename below (the staging + # directory is a sibling inside the same Agent root), so the identity + # recorded in the publication manifest stays exact. _lcs_inode="$(skill_link_inode "$_lcs_staged")" || { restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true rm -rf "$_lcs_stage" return 1 } - # Hard-link the staged symlink itself. This never overwrites a - # concurrently-created destination. If that destination became a - # directory, the unique staged basename can only be linked inside it; - # post-validation detects that case and removes only our link. - if ! ln -P "$_lcs_staged" "$_lcs_dest" 2>/dev/null || ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then + # Anything still present here appeared after the pre-flight scan, i.e. a + # concurrent writer. Never let `mv` clobber it. + if [ -e "$_lcs_dest" ] || [ -L "$_lcs_dest" ]; then + restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true + rm -rf "$_lcs_stage" + return 1 + fi + # Publish by moving the staged symlink. `mv` is POSIX; `ln -P` is not (it is + # absent from BusyBox `ln`, which silently degraded every non-universal + # Agent to the copy layout on Alpine and most containers). install-event.sh + # and install-devapp.sh already publish this way. The staged basename stays + # unique, so a destination a concurrent process turned into a directory can + # only receive our link and post-validation removes exactly that. + if ! mv "$_lcs_staged" "$_lcs_dest" 2>/dev/null || ! skill_link_matches "$_lcs_dest" "$_lcs_link_target" "$_lcs_inode"; then cleanup_nested_staged_link "$_lcs_dest" "$_lcs_stage_name" "$_lcs_link_target" "$_lcs_inode" || true restore_linked_skill_set "$_lcs_published" "$_lcs_backups" || true rm -rf "$_lcs_stage" @@ -900,6 +968,7 @@ install_skills_to_homes() { installed=0 attempted=1 failed=0 + retire_failed=0 if _install_mono_to_base "$skill_src" "$root/.agents/skills" "~/.agents/skills/$SKILL_NAME"; then installed=1; else failed=1; fi [ "$installed" -gt 0 ] || { say " ⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败"; return 1; } for agent_dir in $(agent_skill_dirs) @@ -909,7 +978,7 @@ install_skills_to_homes() { same_physical_skill_root "$base_dir" "$root/.agents/skills" && continue attempted=$((attempted + 1)) if is_universal_agent_dir "$agent_dir"; then - retire_agent_skill_root "$root" "$base_dir" || failed=$((failed + 1)) + retire_agent_skill_root "$root" "$base_dir" || retire_failed=$((retire_failed + 1)) continue fi if link_canonical_skills_to_base "$root" "$base_dir" mono; then @@ -927,6 +996,9 @@ install_skills_to_homes() { say " ⚠️ 未安装任何 mono Skill:所有检测到的 Agent 目标均失败" return 1 fi + if [ "$retire_failed" -gt 0 ]; then + printf ' ⚠️ 有 %s 个 Agent 旧副本未能迁移(安装已完成,可稍后手动删除)\n' "$retire_failed" + fi if [ "$failed" -gt 0 ]; then say " ⚠️ 有 ${failed} 个 Agent 目标安装 mono Skill 失败" return 1 @@ -964,6 +1036,7 @@ install_multi_skills_to_homes() { installed=0 attempted=1 failed=0 + retire_failed=0 if _install_multi_to_base "$multi_src" "$root/.agents/skills" "$root" ".agents/skills"; then installed=1; else failed=1; fi [ "$installed" -gt 0 ] || { say " ⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败"; return 1; } for agent_dir in $(agent_skill_dirs) @@ -973,10 +1046,10 @@ install_multi_skills_to_homes() { same_physical_skill_root "$base_dir" "$root/.agents/skills" && continue attempted=$((attempted + 1)) if is_universal_agent_dir "$agent_dir"; then - retire_agent_skill_root "$root" "$base_dir" || failed=$((failed + 1)) + retire_agent_skill_root "$root" "$base_dir" || retire_failed=$((retire_failed + 1)) continue fi - if link_canonical_skills_to_base "$root" "$base_dir" multi; then + if link_canonical_skills_to_base "$root" "$base_dir" multi "$multi_src"; then installed=$((installed + 1)) else if _install_multi_to_base "$multi_src" "$base_dir" "$root" "$agent_dir"; then @@ -991,6 +1064,9 @@ install_multi_skills_to_homes() { say " ⚠️ 未安装任何 multi Skill:所有检测到的 Agent 目标均失败" return 1 fi + if [ "$retire_failed" -gt 0 ]; then + printf ' ⚠️ 有 %s 个 Agent 旧副本未能迁移(安装已完成,可稍后手动删除)\n' "$retire_failed" + fi if [ "$failed" -gt 0 ]; then say " ⚠️ 有 ${failed} 个 Agent 目标安装失败" return 1 @@ -1335,6 +1411,10 @@ main() { install_skills fi + # Every transaction of this run has finished, so old stamped archives can no + # longer be needed for a rollback. + prune_skill_backups + printf '\n' say "🎉 Installation complete!" say "" diff --git a/scripts/release/verify-package-managers.sh b/scripts/release/verify-package-managers.sh index 3e2cd6b4..da1512b7 100755 --- a/scripts/release/verify-package-managers.sh +++ b/scripts/release/verify-package-managers.sh @@ -223,6 +223,13 @@ verify_compatible_skill_base() { name=$(basename "$(dirname "$skill_md")") target="$base/$name" if [ -L "$target" ]; then + link_target="$(readlink "$target")" || err "could not read canonical Skill link at $target" + # Relative link targets are the headline invariant of the canonical store: + # an absolute target breaks as soon as the home directory moves or the + # store is relocated, so reject it even though it resolves today. + case "$link_target" in + /*) err "canonical Skill link must be relative at $target (target=$link_target)" ;; + esac linked_real="$(CDPATH= cd -- "$target" 2>/dev/null && pwd -P)" || err "broken canonical Skill link at $target" canonical_real="$(CDPATH= cd -- "$canonical/$name" && pwd -P)" || err "canonical Skill missing at $canonical/$name" [ "$linked_real" = "$canonical_real" ] || err "Skill link does not resolve to canonical source at $target" @@ -235,6 +242,23 @@ verify_compatible_skill_base() { [ ! -e "$base/dws" ] && [ ! -L "$base/dws" ] || err "unexpected mono Skill layout found in $base/dws" } +# Universal Agents read the canonical store directly, so they must own no copy +# of ANY canonical Skill. Iterate the real store instead of a hardcoded name +# list: a hardcoded list silently passed leftover copies of every other +# dingtalk-* Skill from earlier betas. +verify_universal_skill_base() { + home_root="$1"; base="$2"; canonical="$home_root/.agents/skills" + for skill_md in "$canonical"/*/SKILL.md; do + [ -f "$skill_md" ] || continue + name=$(basename "$(dirname "$skill_md")") + { [ ! -e "$base/$name" ] && [ ! -L "$base/$name" ]; } || \ + err "unexpected duplicate Skill found in universal Agent root $base/$name" + done + # The mono layout is absent from a multi canonical store, so gate it directly. + { [ ! -e "$base/dws" ] && [ ! -L "$base/dws" ]; } || \ + err "unexpected duplicate Skill found in universal Agent root $base/dws" +} + verify_skill_targets() { home_root="$1" # The universal-convention canonical store is always present, regardless of @@ -253,10 +277,7 @@ verify_skill_targets() { [ "$base" != "$home_root/.agents/skills" ] || continue [ -d "${base%/skills}" ] || continue if [ "$universal" -eq 1 ]; then - for name in dingtalk-shared dingtalk-misc dws; do - { [ ! -e "$base/$name" ] && [ ! -L "$base/$name" ]; } || \ - err "unexpected duplicate Skill found in universal Agent root $base/$name" - done + verify_universal_skill_base "$home_root" "$base" continue fi verify_compatible_skill_base "$home_root" "$base" diff --git a/test/scripts/install_js_smoke.mjs b/test/scripts/install_js_smoke.mjs index 8142f74e..6af14c7a 100644 --- a/test/scripts/install_js_smoke.mjs +++ b/test/scripts/install_js_smoke.mjs @@ -16,22 +16,37 @@ * skills, mono leftover dws/ and stale * dingtalk-* are removed, and the * ~/.dws/skills/{multi,mono} caches fill. - * 2. empty multi/ tree — warns and falls back to mono instead of + * 2. universal topology — canonical store installed, pre-existing private + * copies retired and recoverable under + * ~/.dws/skill-backups//. + * 3. second run — idempotent: an already-correct link is neither + * re-backed-up nor rewritten. + * 4. unretireable universal root — a cleanup-only failure WARNS; canonical + * store, non-universal links and skills-state.json + * are all still written and the install exits 0. + * 5. empty multi/ tree — warns and falls back to mono instead of * crashing postinstall; a previously good multi * cache is NOT wiped. - * 3. bogus mode — DWS_SKILL_MODE=bogus exits non-zero with an + * 6. bogus mode — DWS_SKILL_MODE=bogus exits non-zero with an * "invalid DWS_SKILL_MODE" error. - * 4. multi-only zip, mono — mono install is skipped with a warning; the + * 7. multi-only zip, mono — mono install is skipped with a warning; the * staging root is NOT copied into a dws/ dir. - * 5. multi backup failure — preserves mono, writes no multi skill, and + * 8. multi backup failure — preserves mono, writes no multi skill, and * reports postinstall failure. - * 6. mono backup failure — preserves multi, writes no mono skill, and + * 9. mono backup failure — preserves multi, writes no mono skill, and * reports postinstall failure. - * 7. mono switch — migrates only centrally owned multi Skills. - * 8. cache copy failure — preserves the previous complete cache. - * 9. multi publish failure — restores the complete previous Skill set. - * 10. multi backup failure — restores every earlier successful backup. - * 11. mono transaction failure — restores every managed multi Skill after + * 10. mono switch — migrates only centrally owned multi Skills. + * 11. cache copy failure — preserves the previous complete cache. + * 12. no-replace publish — an occupied destination that is not a victim is + * refused, not replaced or linked into. + * 13. simulated win32 — junction type + ABSOLUTE canonical target and + * the rename publish path with its no-replace + * guard, exercised by swapping process.platform + * instead of requiring a Windows host. + * 14. backup retention — ~/.dws/skill-backups keeps the newest 5 stamps. + * 15. multi publish failure — restores the complete previous Skill set. + * 16. multi backup failure — restores every earlier successful backup. + * 17. mono transaction failure — restores every managed multi Skill after * later backup or mono publish failure. * * Requirements: unix host with tar/zip/unzip on PATH (the same tools @@ -133,6 +148,24 @@ function crossDeviceError() { return Object.assign(new Error("injected cross-device rename"), { code: "EXDEV" }); } +// withSimulatedWin32 runs fn with process.platform === "win32" so the +// Windows-only publish branches (junction type, absolute link target, rename +// publish + its no-replace guard) get real execution coverage on a unix host. +// fs.symlinkSync ignores the type argument off Windows, so the same call is +// safe here. Create every temp path BEFORE calling: os.tmpdir() follows the +// simulated platform. +function withSimulatedWin32(fn) { + const original = process.platform; + const set = (value) => + Object.defineProperty(process, "platform", { value, configurable: true, enumerable: true, writable: false }); + set("win32"); + try { + return fn(); + } finally { + set(original); + } +} + function runCrossDeviceMoveContract() { const roots = []; const fixture = (name) => { @@ -465,6 +498,93 @@ scenario("ZCode links its Agent root to the canonical store", () => { } }); +scenario("a second install run is idempotent and preserves correct links", () => { + const { tmp, pkg, home } = stagePkg({ + "mono/SKILL.md": "# mono fixture\n", + "multi/dingtalk-chat/SKILL.md": "# dingtalk-chat\n", + }); + try { + writeFile(path.join(home, ".zcode", "v2", "config.json"), "{}\n"); + + const first = runInstall(pkg, home, "multi", { XDG_CONFIG_HOME: path.join(home, ".config") }); + assert.equal(first.status, 0, `exit=${first.status}\nstdout=${first.stdout}\nstderr=${first.stderr}`); + const linked = path.join(home, ".zcode", "skills", "dingtalk-chat"); + const firstTarget = fs.readlinkSync(linked); + const firstState = JSON.parse(fs.readFileSync(path.join(home, ".dws", "skills-state.json"), "utf8")); + + const second = runInstall(pkg, home, "multi", { XDG_CONFIG_HOME: path.join(home, ".config") }); + assert.equal(second.status, 0, `exit=${second.status}\nstdout=${second.stdout}\nstderr=${second.stderr}`); + assert.equal(fs.lstatSync(linked).isSymbolicLink(), true, "an already-correct link stays a link"); + assert.equal(fs.readlinkSync(linked), firstTarget, "an already-correct link is not rewritten"); + assert.equal(fs.readFileSync(path.join(linked, "SKILL.md"), "utf8"), "# dingtalk-chat\n"); + assert.ok( + !second.stdout.includes(`已备份并移除 ${linked}`), + "an already-correct link must not be backed up again", + ); + const secondState = JSON.parse(fs.readFileSync(path.join(home, ".dws", "skills-state.json"), "utf8")); + assert.deepEqual(secondState.official_skills, firstState.official_skills, "state is stable across runs"); + assert.deepEqual( + secondState.managed_skills.map((record) => record.digest), + firstState.managed_skills.map((record) => record.digest), + "re-installing the same bundle yields the same provenance digests", + ); + assert.ok( + fs.readdirSync(path.join(home, ".dws", "skill-backups")).length <= 5, + "repeated installs keep the backup root bounded", + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); + +scenario("unretireable universal copy warns but still completes the install", () => { + const { tmp, pkg, home } = stagePkg({ + "mono/SKILL.md": "# mono fixture\n", + "multi/dingtalk-chat/SKILL.md": "# dingtalk-chat\n", + }); + const codexSkills = path.join(home, ".codex", "skills"); + try { + // Non-universal Agent that must still receive its link. + writeFile(path.join(home, ".zcode", "v2", "config.json"), "{}\n"); + // Universal Agent whose obsolete copy cannot be moved out: r-x on the + // parent means rename() fails with EACCES. + writeFile(path.join(codexSkills, "dingtalk-chat", "SKILL.md"), "old codex copy\n"); + fs.chmodSync(codexSkills, 0o500); + + const res = runInstall(pkg, home, "multi", { + XDG_CONFIG_HOME: path.join(home, ".config"), + CODEX_HOME: "", + }); + assert.equal( + res.status, + 0, + `a cleanup-only failure must not fail the install\nexit=${res.status}\nstdout=${res.stdout}\nstderr=${res.stderr}`, + ); + assert.match(res.stderr, /旧副本迁移失败/, "the retire failure is reported as a warning"); + assert.ok( + fs.existsSync(path.join(home, ".agents", "skills", "dingtalk-chat", "SKILL.md")), + "canonical store is still installed", + ); + const linked = path.join(home, ".zcode", "skills", "dingtalk-chat"); + assert.equal(fs.lstatSync(linked).isSymbolicLink(), true, "non-universal link is still installed"); + assert.equal(fs.readFileSync(path.join(linked, "SKILL.md"), "utf8"), "# dingtalk-chat\n"); + assert.ok( + fs.existsSync(path.join(home, ".dws", "skills-state.json")), + "skills-state.json is written even when a retire failed", + ); + assert.equal( + fs.readFileSync(path.join(codexSkills, "dingtalk-chat", "SKILL.md"), "utf8"), + "old codex copy\n", + "a copy that cannot be backed up is never deleted", + ); + } finally { + try { + fs.chmodSync(codexSkills, 0o700); + } catch {} + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); + scenario("upstream Agent roots honor XDG and custom homes with relative links", () => { const { tmp, pkg, home } = stagePkg({ "mono/SKILL.md": "# mono fixture\n", @@ -510,16 +630,28 @@ scenario("upstream Agent roots honor XDG and custom homes with relative links", assert.equal(fs.readFileSync(path.join(linked, "SKILL.md"), "utf8"), "# dingtalk-chat\n"); } const backupText = []; + const backupNames = []; const collectBackupText = (dir) => { for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { const full = path.join(dir, entry.name); - if (entry.isDirectory()) collectBackupText(full); - else if (entry.isFile()) backupText.push(fs.readFileSync(full, "utf8")); + if (entry.isDirectory()) { + backupNames.push(entry.name); + collectBackupText(full); + } else if (entry.isFile()) backupText.push(fs.readFileSync(full, "utf8")); } }; collectBackupText(path.join(home, ".dws", "skill-backups")); assert.ok(backupText.includes("old claude copy\n"), "CLAUDE_CONFIG_DIR content is recoverably backed up"); assert.ok(backupText.includes("old hermes copy\n"), "HERMES_HOME content is recoverably backed up"); + // Retired universal copies are the PR's advertised recovery story: every + // one of them must be readable back out of ~/.dws/skill-backups. + assert.ok(backupText.includes("old amp copy\n"), "retired XDG agents copy is recoverable"); + assert.ok(backupText.includes("old codex copy\n"), "retired CODEX_HOME copy is recoverable"); + assert.ok(backupText.includes("old DWS path\n"), "retired legacy ~/.amp copy is recoverable"); + assert.ok( + backupNames.includes(".amp-skills-dingtalk-chat"), + `backup names record the HOME-relative origin, got ${backupNames.join(",")}`, + ); for (const retired of [ path.join(xdg, "agents", "skills", "dingtalk-chat"), path.join(codex, "skills", "dingtalk-chat"), @@ -691,21 +823,31 @@ scenario("canonical link publication never clobbers or deletes concurrent user d const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "dws-installjs-canonical-race-")); try { { + // Real no-replace property: the destination is occupied by data this + // transaction never backed up (it is not a victim), so the production + // publisher must refuse rather than replace or link into it. No stub + // creates or destroys anything here. const home = path.join(tmp, "no-clobber-home"); const canonical = path.join(home, ".agents", "skills"); const base = path.join(home, ".cursor", "skills"); const destination = path.join(base, "dingtalk-chat"); writeFile(path.join(canonical, "dingtalk-chat", "SKILL.md"), "new\n"); + writeFile(path.join(destination, "concurrent-user-data.txt"), "must survive\n"); assert.throws( - () => publishCanonicalLinksAtomically(home, canonical, base, ["dingtalk-chat"], [], { - publishLinkFn(_source, target) { - writeFile(path.join(target, "concurrent-user-data.txt"), "must survive\n"); - throw Object.assign(new Error("concurrent destination exists"), { code: "EEXIST" }); - }, - }), - /concurrent destination exists/, + () => publishCanonicalLinksAtomically(home, canonical, base, ["dingtalk-chat"], []), + /Skill publish destination already exists/, ); assert.equal(fs.readFileSync(path.join(destination, "concurrent-user-data.txt"), "utf8"), "must survive\n"); + assert.deepEqual( + fs.readdirSync(destination), + ["concurrent-user-data.txt"], + "refused publication must not leave anything inside the occupied destination", + ); + assert.equal(fs.lstatSync(destination).isDirectory(), true, "the user's directory is still a directory"); + assert.ok( + !fs.readdirSync(base).some((name) => name.startsWith(".dws-link-set.tmp-")), + "refused publication cleans its staging root", + ); } { @@ -756,6 +898,111 @@ scenario("canonical link publication never clobbers or deletes concurrent user d } }); +scenario("simulated win32 publishes junctions with absolute canonical targets", () => { + // Temp roots are created before entering the simulated platform: os.tmpdir() + // would otherwise resolve through Windows environment variables. + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "dws-installjs-win32-link-")); + try { + const home = path.join(tmp, "home"); + const canonical = path.join(home, ".agents", "skills"); + const canonicalSkill = path.join(canonical, "dingtalk-chat"); + const base = path.join(home, ".zcode", "skills"); + const dest = path.join(base, "dingtalk-chat"); + writeFile(path.join(canonicalSkill, "SKILL.md"), "# dingtalk-chat\n"); + writeFile(path.join(dest, "SKILL.md"), "old zcode copy\n"); + + const symlinkCalls = []; + const renameCalls = []; + withSimulatedWin32(() => { + publishCanonicalLinksAtomically(home, canonical, base, ["dingtalk-chat"], [dest], { + symlinkFn(target, linkPath, type) { + symlinkCalls.push({ target, linkPath, type }); + fs.symlinkSync(target, linkPath, type); + }, + publishRenameFn(source, destination) { + renameCalls.push({ source, destination }); + fs.renameSync(source, destination); + }, + }); + }); + + assert.equal(symlinkCalls.length, 1, "one staged link per bundle skill"); + assert.equal(symlinkCalls[0].type, "junction", "Windows must publish junctions, not symlinks"); + assert.equal(path.isAbsolute(symlinkCalls[0].target), true, "junctions require an absolute target"); + assert.equal(symlinkCalls[0].target, fs.realpathSync(canonicalSkill), "junction targets the canonical store"); + assert.equal(renameCalls.length, 1, "win32 publishes by rename, not by ln -P"); + assert.equal(renameCalls[0].destination, dest); + assert.equal(fs.readlinkSync(dest), fs.realpathSync(canonicalSkill), "published junction kept its absolute target"); + assert.equal(fs.readFileSync(path.join(dest, "SKILL.md"), "utf8"), "# dingtalk-chat\n"); + const backupRoot = path.join(home, ".dws", "skill-backups"); + const stamps = fs.readdirSync(backupRoot); + assert.equal(stamps.length, 1, "the replaced copy produced exactly one backup stamp"); + assert.deepEqual( + fs.readdirSync(path.join(backupRoot, stamps[0])), + [".zcode-skills-dingtalk-chat"], + "backups encode the HOME-relative origin of the retired copy", + ); + assert.equal( + fs.readFileSync(path.join(backupRoot, stamps[0], ".zcode-skills-dingtalk-chat", "SKILL.md"), "utf8"), + "old zcode copy\n", + "the replaced copy stays recoverable", + ); + + // FIX 5: the win32 rename path must never replace an object that appeared + // at the destination (libuv passes MOVEFILE_REPLACE_EXISTING). + const raceBase = path.join(home, ".qoder", "skills"); + const occupied = path.join(raceBase, "dingtalk-chat"); + writeFile(path.join(occupied, "concurrent-user-data.txt"), "must survive\n"); + let renames = 0; + withSimulatedWin32(() => { + assert.throws( + () => publishCanonicalLinksAtomically(home, canonical, raceBase, ["dingtalk-chat"], [], { + publishRenameFn(source, destination) { + renames += 1; + fs.renameSync(source, destination); + }, + }), + /Skill publish destination already exists/, + ); + }); + assert.equal(renames, 0, "win32 publish must not rename over an existing destination"); + assert.deepEqual(fs.readdirSync(occupied), ["concurrent-user-data.txt"]); + assert.equal(fs.readFileSync(path.join(occupied, "concurrent-user-data.txt"), "utf8"), "must survive\n"); + assert.equal(process.platform !== "win32", true, "the simulated platform is restored"); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); + +scenario("skill backups stay bounded to the newest five stamps", () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "dws-installjs-backup-prune-")); + const home = path.join(tmp, "home"); + const backupRoot = path.join(home, ".dws", "skill-backups"); + try { + for (const stamp of [ + "20260101-000001", "20260101-000002", "20260101-000003", + "20260101-000004", "20260101-000005", "20260101-000006", "20260101-000007", + ]) { + writeFile(path.join(backupRoot, stamp, "placeholder", "SKILL.md"), `${stamp}\n`); + } + const victim = path.join(home, ".codex", "skills", "dingtalk-chat"); + writeFile(path.join(victim, "SKILL.md"), "old codex copy\n"); + + const backup = backupAndRemoveSkillDir(home, victim, null, { backupStampFn: () => "20260102-000000" }); + + const stamps = fs.readdirSync(backupRoot).sort(); + assert.equal(stamps.length, 5, `backup root must stay bounded, got ${stamps.join(",")}`); + assert.ok(stamps.includes("20260102-000000"), "the backup from this run survives pruning"); + assert.ok(!stamps.includes("20260101-000001"), "the oldest stamps are pruned first"); + assert.ok(!stamps.includes("20260101-000003"), "pruning removes exactly the excess"); + assert.ok(stamps.includes("20260101-000004"), "the newest kept stamps are retained"); + assert.equal(path.basename(backup), ".codex-skills-dingtalk-chat"); + assert.equal(fs.readFileSync(path.join(backup, "SKILL.md"), "utf8"), "old codex copy\n"); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); + scenario("multi set publish failure restores the complete previous set", () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "dws-installjs-multi-set-")); const home = path.join(tmp, "home"); diff --git a/test/scripts/install_script_test.go b/test/scripts/install_script_test.go index 006cabc2..483840aa 100644 --- a/test/scripts/install_script_test.go +++ b/test/scripts/install_script_test.go @@ -1862,18 +1862,24 @@ func TestInstallerShellLinkPublicationRacePreservesConcurrentDirectories(t *test second := filepath.Join(base, "dingtalk-second") mustWriteFile(t, filepath.Join(home, ".agents", "skills", "dingtalk-first", "SKILL.md"), []byte("first\n"), 0o644) mustWriteFile(t, filepath.Join(home, ".agents", "skills", "dingtalk-second", "SKILL.md"), []byte("second\n"), 0o644) + bundle := filepath.Join(t.TempDir(), "multi") + mustWriteFile(t, filepath.Join(bundle, "dingtalk-first", "SKILL.md"), []byte("first\n"), 0o644) + mustWriteFile(t, filepath.Join(bundle, "dingtalk-second", "SKILL.md"), []byte("second\n"), 0o644) + // Publication now moves a staged symlink instead of using `ln -P` + // (BusyBox has no -P), and multi mode requires the bundle source so + // that only bundle skills are linked. Inject the race on that move. harness := `. "$DWS_TEST_LIBRARY" -ln() { - if [ "$1" = -P ] && [ "$3" = "$DWS_TEST_SECOND" ]; then +mv() { + if [ "$2" = "$DWS_TEST_SECOND" ]; then rm -f "$DWS_TEST_FIRST" mkdir -p "$DWS_TEST_FIRST" "$DWS_TEST_SECOND" printf '%s\n' first-user-data > "$DWS_TEST_FIRST/user.txt" printf '%s\n' second-user-data > "$DWS_TEST_SECOND/user.txt" fi - command ln "$@" + command mv "$@" } -if link_canonical_skills_to_base "$HOME" "$DWS_TEST_BASE" multi; then +if link_canonical_skills_to_base "$HOME" "$DWS_TEST_BASE" multi "$DWS_TEST_BUNDLE"; then exit 2 fi identity_dest="$DWS_TEST_BASE/identity-link" @@ -1895,6 +1901,7 @@ rm -f "$identity_anchor" "HOME="+home, "DWS_TEST_LIBRARY="+library, "DWS_TEST_BASE="+base, + "DWS_TEST_BUNDLE="+bundle, "DWS_TEST_FIRST="+first, "DWS_TEST_SECOND="+second, ) diff --git a/test/scripts/package_script_test.go b/test/scripts/package_script_test.go index 97ee3235..46155da0 100644 --- a/test/scripts/package_script_test.go +++ b/test/scripts/package_script_test.go @@ -180,9 +180,11 @@ func TestPackageManagerVerifierAcceptsOnlyCorrectLinksOrCopies(t *testing.T) { }{ { name: "correct-link", - prepare: func(t *testing.T, home, base string) { + prepare: func(t *testing.T, _, base string) { t.Helper() - if err := os.Symlink(filepath.Join(home, ".agents", "skills", "dingtalk-shared"), filepath.Join(base, "dingtalk-shared")); err != nil { + // The canonical layout publishes RELATIVE links; the verifier now + // rejects absolute targets, so the fixture must match production. + if err := os.Symlink(filepath.Join("..", "..", ".agents", "skills", "dingtalk-shared"), filepath.Join(base, "dingtalk-shared")); err != nil { t.Fatal(err) } },