diff --git a/build/npm/install.js b/build/npm/install.js index 882a17c5..ac74bb8d 100755 --- a/build/npm/install.js +++ b/build/npm/install.js @@ -163,6 +163,31 @@ const SKILL_BACKUP_KEEP = 5; // transaction still needs them to roll back. const currentRunBackupRoots = new Set(); +// Ownership marker: every stamp root DWS creates carries .dws-skill-backup +// with exactly these bytes — the same bytes internal/upgrade/paths.go, +// install.sh, and the PowerShell installers write. A stamp-shaped name alone +// is not ownership proof, so pruning only deletes directories whose marker +// content matches byte-for-byte (cross-surface exact-LF contract; a +// PowerShell writer's hypothetical CRLF is normalized away on that surface, +// while every writer here emits LF). +const SKILL_BACKUP_MARKER_FILE = ".dws-skill-backup"; +const SKILL_BACKUP_MARKER_BODY = "dws skill backup v1\n"; + +// writeSkillBackupMarker stamps a freshly created stamp root as DWS-owned. +function writeSkillBackupMarker(root) { + fs.writeFileSync(path.join(root, SKILL_BACKUP_MARKER_FILE), SKILL_BACKUP_MARKER_BODY); +} + +// isProvenSkillBackupRoot reports whether a stamp root carries the ownership +// marker with the exact expected bytes; anything else is foreign data. +function isProvenSkillBackupRoot(root) { + try { + return fs.readFileSync(path.join(root, SKILL_BACKUP_MARKER_FILE), "utf8") === SKILL_BACKUP_MARKER_BODY; + } catch { + return false; + } +} + // skillBackupStampPattern matches only directory names DWS itself writes: // UTC YYYYmmdd-HHMMSS, with an optional -N collision suffix. Any other entry // in the backup root is foreign (user data, unrelated tooling) and must be @@ -175,9 +200,10 @@ function isSkillBackupStamp(name) { // pruneSkillBackups removes the oldest stamped backup directories once more // than SKILL_BACKUP_KEEP remain. Only directories whose names match the DWS -// backup stamp format are candidates; unknown directories are preserved. -// Stamps sort lexicographically in chronological order (`YYYYmmdd-HHMMSS`), -// so name order is age order. +// backup stamp format AND whose ownership marker verifies are candidates; +// unknown or unmarked directories are foreign data — preserved and never +// counted against SKILL_BACKUP_KEEP. 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; @@ -189,6 +215,7 @@ function pruneSkillBackups(homeDir) { } const names = entries .filter((entry) => entry.isDirectory() && isSkillBackupStamp(entry.name)) + .filter((entry) => isProvenSkillBackupRoot(path.join(root, entry.name))) .map((entry) => entry.name) .sort((a, b) => Buffer.from(a).compare(Buffer.from(b))); let excess = names.length - SKILL_BACKUP_KEEP; @@ -587,6 +614,23 @@ function backupAndRemoveSkillDir(homeDir, dir, backups = null, options = {}) { } try { fs.mkdirSync(targetRoot, { recursive: true }); + // Stamp ownership immediately after creating the stamp root and before + // the skill directory moves into it, so an interrupted backup can never + // leave an unmarked (never-prunable) stamp behind. + writeSkillBackupMarker(targetRoot); + } catch (err) { + // Never leave an empty unowned stamp root behind. The removal is + // deliberately non-recursive so a pre-existing non-empty root (foreign + // data) is never destroyed. + try { + fs.rmSync(targetRoot, { force: true }); + } catch { + // Non-empty pre-existing root: foreign data stays. + } + console.warn(`⚠️ 备份失败,保留原目录 ${dir}: ${err.message}`); + throw new Error(`failed to back up Skill directory ${dir}: ${err.message}`); + } + try { movePathRecoverablySync(dir, target, options); } catch (err) { console.warn(`⚠️ 备份失败,保留原目录 ${dir}: ${err.message}`); diff --git a/internal/upgrade/paths.go b/internal/upgrade/paths.go index 349b156c..88df2324 100644 --- a/internal/upgrade/paths.go +++ b/internal/upgrade/paths.go @@ -151,6 +151,8 @@ var ( upgradeMkdirAll = os.MkdirAll upgradeMkdirTemp = os.MkdirTemp upgradeReadDir = os.ReadDir + upgradeReadFile = os.ReadFile + upgradeWriteFile = os.WriteFile upgradeStat = os.Stat upgradeLstat = os.Lstat upgradeSymlink = os.Symlink @@ -172,6 +174,28 @@ var ( // ask for confirmation, so deletions must stay reversible instead. const skillBackupSubdir = ".dws/skill-backups" +// skillBackupMarkerFile is the ownership proof written into every stamp root +// DWS creates. A stamp-shaped name alone is not evidence: users and other +// tools can legitimately write `YYYYMMDD-HHMMSS` directories into the backup +// root, so pruning only deletes directories whose marker content matches. +// Installers on every surface (Go, sh, PowerShell, npm) write and verify the +// exact same bytes. +const skillBackupMarkerFile = ".dws-skill-backup" + +const skillBackupMarkerBody = "dws skill backup v1\n" + +// writeSkillBackupMarker stamps a freshly created stamp root as DWS-owned. +func writeSkillBackupMarker(root string) error { + return upgradeWriteFile(filepath.Join(root, skillBackupMarkerFile), []byte(skillBackupMarkerBody), 0o644) +} + +// isProvenSkillBackupRoot reports whether root carries the DWS ownership +// marker with the exact expected content. +func isProvenSkillBackupRoot(root string) bool { + body, err := upgradeReadFile(filepath.Join(root, skillBackupMarkerFile)) + return err == nil && string(body) == skillBackupMarkerBody +} + // backupAndRemoveSkillDir moves dir into /.dws/skill-backups/ // / instead of destroying it, and returns the backup path. It is // fail-safe: a path that cannot be backed up is NOT removed and the @@ -212,6 +236,10 @@ func backupAndRemoveSkillDir(homeDir, dir string) (string, error) { if err := upgradeMkdirAll(backupRoot, dirPermShared); err != nil { return "", fmt.Errorf("创建备份目录失败 %s: %w", backupRoot, err) } + if err := writeSkillBackupMarker(backupRoot); err != nil { + _ = upgradeRemoveAll(backupRoot) + return "", fmt.Errorf("写入备份所有权标记失败 %s: %w", backupRoot, err) + } if err := moveSkillPathRecoverably(dir, target); err != nil { return "", fmt.Errorf("备份技能目录失败 %s: %w", dir, err) } @@ -422,9 +450,11 @@ func isSkillBackupRunRoot(path string) bool { // pruneSkillBackups removes the oldest backup directories when more than // skillBackupKeep remain. Only directories whose names match the DWS backup -// stamp format are candidates; unknown directories and stamps created by the -// current run are preserved. Best-effort: a removal failure never aborts, but -// pruning failures are reported so callers can warn the user. +// stamp format AND whose ownership marker verifies are candidates; a +// stamp-shaped name alone is not proof of ownership, so unmarked directories +// are neither counted nor deleted, and stamps created by the current run are +// preserved. Best-effort: a removal failure never aborts, but pruning +// failures are reported so callers can warn the user. func pruneSkillBackups(homeDir string) error { root := filepath.Join(homeDir, skillBackupSubdir) entries, err := upgradeReadDir(root) @@ -436,9 +466,13 @@ func pruneSkillBackups(homeDir string) error { } names := make([]string, 0, len(entries)) for _, e := range entries { - if e.IsDir() && isSkillBackupStamp(e.Name()) { - names = append(names, e.Name()) + if !e.IsDir() || !isSkillBackupStamp(e.Name()) { + continue } + if !isProvenSkillBackupRoot(filepath.Join(root, e.Name())) { + continue + } + names = append(names, e.Name()) } sort.Strings(names) var firstErr error diff --git a/internal/upgrade/paths_multi_test.go b/internal/upgrade/paths_multi_test.go index ddd0f238..2790bdf3 100644 --- a/internal/upgrade/paths_multi_test.go +++ b/internal/upgrade/paths_multi_test.go @@ -824,6 +824,17 @@ func TestCrossPlatformCoverageBackupAndRemoveSkillDirEdges(t *testing.T) { testseam.Swap(t, &skillPathRenameNoReplace, renameSkillPathNoReplace) } +// seedSkillBackupStamp creates a prune-eligible DWS backup stamp directory. +func seedSkillBackupStamp(t *testing.T, root, name string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(root, name), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, name, skillBackupMarkerFile), []byte(skillBackupMarkerBody), 0o644); err != nil { + t.Fatal(err) + } +} + // TestCrossPlatformCoveragePruneKeepsCurrentRunBackups pins the reversibility // guarantee. The backup stamp only has second precision, so a migration that // retires copies across many Agent roots produces far more than @@ -834,9 +845,7 @@ func TestCrossPlatformCoveragePruneKeepsCurrentRunBackups(t *testing.T) { root := filepath.Join(home, skillBackupSubdir) // Pre-existing backups from earlier runs are still prunable. for i := 0; i < 3; i++ { - if err := os.MkdirAll(filepath.Join(root, fmt.Sprintf("20250101-00000%d", i)), 0o755); err != nil { - t.Fatal(err) - } + seedSkillBackupStamp(t, root, fmt.Sprintf("20250101-00000%d", i)) } stamp := 0 testseam.Swap(t, &upgradeBackupStamp, func() string { @@ -868,7 +877,7 @@ func TestCrossPlatformCoveragePruneKeepsCurrentRunBackups(t *testing.T) { for i := 0; i < 3; i++ { stale := filepath.Join(root, fmt.Sprintf("20250101-00000%d", i)) if _, err := os.Stat(stale); !os.IsNotExist(err) { - t.Errorf("foreign backup %s must still be pruned, stat err=%v", stale, err) + t.Errorf("marked stale backup %s must still be pruned, stat err=%v", stale, err) } } } @@ -876,13 +885,92 @@ func TestCrossPlatformCoveragePruneKeepsCurrentRunBackups(t *testing.T) { // TestCrossPlatformCoveragePruneSkillBackupsEdges pins the backup retention: // oldest stamps beyond skillBackupKeep are pruned and a prune failure is // reported without aborting callers. +// TestCrossPlatformCoverageSkillBackupMarkerOwnership pins the prune ownership +// contract: backups carry the DWS marker, a mismatched marker is foreign data, +// and a backup whose marker cannot be written never removes the source. +func TestCrossPlatformCoverageSkillBackupMarkerOwnership(t *testing.T) { + t.Run("backup creation writes the ownership marker", func(t *testing.T) { + home := t.TempDir() + victim := filepath.Join(home, ".claude", "skills", "dingtalk-chat") + if err := os.MkdirAll(victim, 0o755); err != nil { + t.Fatal(err) + } + backup, err := backupAndRemoveSkillDir(home, victim) + if err != nil { + t.Fatal(err) + } + body, readErr := os.ReadFile(filepath.Join(filepath.Dir(backup), skillBackupMarkerFile)) + if readErr != nil || string(body) != skillBackupMarkerBody { + t.Fatalf("backup marker = %q, %v", body, readErr) + } + }) + + t.Run("stamp with mismatched marker content is preserved", func(t *testing.T) { + home := t.TempDir() + root := filepath.Join(home, skillBackupSubdir) + for i := 0; i < skillBackupKeep+1; i++ { + seedSkillBackupStamp(t, root, fmt.Sprintf("20261210-00000%d", i)) + } + forged := filepath.Join(root, "20261210-999999") + if err := os.MkdirAll(forged, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(forged, skillBackupMarkerFile), []byte("forged\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := pruneSkillBackups(home); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(forged); err != nil { + t.Fatalf("forged-marker backup must be preserved: %v", err) + } + }) + + t.Run("unreadable marker is treated as foreign", func(t *testing.T) { + home := t.TempDir() + root := filepath.Join(home, skillBackupSubdir) + for i := 0; i < skillBackupKeep+1; i++ { + seedSkillBackupStamp(t, root, fmt.Sprintf("20261211-00000%d", i)) + } + testseam.Swap(t, &upgradeReadFile, func(string) ([]byte, error) { return nil, errors.New("read denied") }) + if err := pruneSkillBackups(home); err != nil { + t.Fatalf("prune with unreadable markers = %v", err) + } + testseam.Swap(t, &upgradeReadFile, os.ReadFile) + entries, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + if len(entries) != skillBackupKeep+1 { + t.Fatalf("unreadable markers must disable pruning, entries = %d", len(entries)) + } + }) + + t.Run("marker write failure keeps the source in place", func(t *testing.T) { + home := t.TempDir() + victim := filepath.Join(home, ".claude", "skills", "dingtalk-chat") + if err := os.MkdirAll(victim, 0o755); err != nil { + t.Fatal(err) + } + testseam.Swap(t, &upgradeWriteFile, func(string, []byte, os.FileMode) error { return errors.New("write denied") }) + backup, err := backupAndRemoveSkillDir(home, victim) + if err == nil || !strings.Contains(err.Error(), "备份所有权标记") { + t.Fatalf("marker write failure must fail the backup, got %v", err) + } + if backup != "" { + t.Fatalf("failed backup must not return a path: %q", backup) + } + if _, statErr := os.Stat(victim); statErr != nil { + t.Fatalf("victim must stay in place: %v", statErr) + } + }) +} + func TestCrossPlatformCoveragePruneSkillBackupsEdges(t *testing.T) { home := t.TempDir() root := filepath.Join(home, skillBackupSubdir) for i := 0; i < skillBackupKeep+2; i++ { - if err := os.MkdirAll(filepath.Join(root, fmt.Sprintf("20260810-00000%d", i)), 0o755); err != nil { - t.Fatal(err) - } + seedSkillBackupStamp(t, root, fmt.Sprintf("20260810-00000%d", i)) } if err := pruneSkillBackups(home); err != nil { t.Fatalf("pruneSkillBackups() error = %v", err) @@ -915,9 +1003,7 @@ func TestCrossPlatformCoveragePruneSkillBackupsEdges(t *testing.T) { // Removal failure is reported as the first error. Seed more than // skillBackupKeep dirs again so the prune loop actually runs. for i := 0; i < skillBackupKeep+2; i++ { - if err := os.MkdirAll(filepath.Join(root, fmt.Sprintf("20260811-00000%d", i)), 0o755); err != nil { - t.Fatal(err) - } + seedSkillBackupStamp(t, root, fmt.Sprintf("20260811-00000%d", i)) } testseam.Swap(t, &upgradeRemoveAll, func(string) error { return errors.New("remove denied") }) if err := pruneSkillBackups(home); err == nil { @@ -928,20 +1014,17 @@ func TestCrossPlatformCoveragePruneSkillBackupsEdges(t *testing.T) { // TestCrossPlatformCoveragePruneSkillBackupsPreservesUnknown pins that // pruneSkillBackups only retires directories whose names match the DWS backup -// stamp format. Any foreign directory in the backup root must survive even when -// more than skillBackupKeep stamped backups exist. +// stamp format and whose ownership marker verifies. Any foreign or unmarked +// directory in the backup root must survive even when more than +// skillBackupKeep stamped backups exist. func TestCrossPlatformCoveragePruneSkillBackupsPreservesUnknown(t *testing.T) { home := t.TempDir() root := filepath.Join(home, skillBackupSubdir) for i := 0; i < skillBackupKeep+2; i++ { - if err := os.MkdirAll(filepath.Join(root, fmt.Sprintf("20260910-00000%d", i)), 0o755); err != nil { - t.Fatal(err) - } + seedSkillBackupStamp(t, root, fmt.Sprintf("20260910-00000%d", i)) } // A collision-suffixed stamp is still a valid DWS backup and is eligible. - if err := os.MkdirAll(filepath.Join(root, "20260910-000000-7"), 0o755); err != nil { - t.Fatal(err) - } + seedSkillBackupStamp(t, root, "20260910-000000-7") unknown := []string{ "user-personal-backup", "20260910-000000-abc", // stamp-shaped but non-numeric suffix @@ -953,12 +1036,18 @@ func TestCrossPlatformCoveragePruneSkillBackupsPreservesUnknown(t *testing.T) { t.Fatal(err) } } + // A stamp-shaped directory without the DWS ownership marker is foreign + // data: never counted toward the keep limit, never deleted. + unmarked := "20991231-235959" + if err := os.MkdirAll(filepath.Join(root, unmarked), 0o755); err != nil { + t.Fatal(err) + } if err := pruneSkillBackups(home); err != nil { t.Fatalf("pruneSkillBackups() error = %v", err) } - for _, name := range unknown { + for _, name := range append(append([]string{}, unknown...), unmarked) { if _, err := os.Stat(filepath.Join(root, name)); err != nil { t.Errorf("unknown backup entry %s must be preserved, stat err=%v", name, err) } @@ -975,8 +1064,8 @@ func TestCrossPlatformCoveragePruneSkillBackupsPreservesUnknown(t *testing.T) { stamps++ } } - if stamps != skillBackupKeep { - t.Errorf("stamped backups after prune = %d, want %d", stamps, skillBackupKeep) + if stamps != skillBackupKeep+1 { + t.Errorf("stamped backups after prune = %d, want %d (kept plus the unmarked foreign one)", stamps, skillBackupKeep+1) } } diff --git a/scripts/install-devapp.ps1 b/scripts/install-devapp.ps1 index 58fa7822..84558494 100644 --- a/scripts/install-devapp.ps1 +++ b/scripts/install-devapp.ps1 @@ -312,14 +312,49 @@ function Get-DevSkillBackupName([string]$Dir) { $DevSkillBackupKeep = 5 $script:DevSkillBackupRootsThisRun = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) +# Ownership marker: every stamp root DWS creates carries .dws-skill-backup +# with exactly "dws skill backup v1" + LF — the same bytes internal/upgrade/ +# paths.go, install.sh, and build/npm/install.js write. A stamp-shaped name +# alone is not ownership proof, so pruning only deletes directories whose +# marker content verifies. +$DevSkillBackupMarkerFile = ".dws-skill-backup" +$DevSkillBackupMarkerBody = "dws skill backup v1" + +# Write-DevSkillBackupMarker stamps a freshly created stamp root as DWS-owned. +# [IO.File]::WriteAllText pins the exact LF-terminated bytes (Set-Content +# would append a platform newline on Windows PowerShell 5.1). +function Write-DevSkillBackupMarker([string]$Root) { + [System.IO.File]::WriteAllText( + (Join-Path $Root $DevSkillBackupMarkerFile), + "$DevSkillBackupMarkerBody`n", + [System.Text.UTF8Encoding]::new($false)) +} + +# Test-DevSkillBackupMarker reports whether a stamp root carries the ownership +# marker. The check normalizes CRLF→LF and drops trailing newlines before +# comparing, so this surface accepts every surface's exact-LF bytes (writer +# and checker agree); any other content, or a missing/unreadable marker, +# means foreign data. +function Test-DevSkillBackupMarker([string]$Dir) { + try { + $marker = Join-Path $Dir $DevSkillBackupMarkerFile + if (![System.IO.File]::Exists($marker)) { return $false } + $body = [System.IO.File]::ReadAllText($marker).Replace("`r`n", "`n").TrimEnd("`r", "`n") + return ($body -eq $DevSkillBackupMarkerBody) + } catch { + return $false + } +} + function Remove-OldDevSkillBackups { $root = Join-Path $HOME ".dws\skill-backups" if (!(Test-Path -LiteralPath $root -PathType Container)) { return } # Only directories whose names match the DWS backup stamp format (UTC - # yyyyMMdd-HHmmss, optional -N collision suffix) are candidates; any - # other entry is foreign data and is preserved. + # yyyyMMdd-HHmmss, optional -N collision suffix) AND whose ownership + # marker verifies are candidates; anything else is foreign data — + # preserved and never counted against $DevSkillBackupKeep. $dirs = @(Get-ChildItem -LiteralPath $root -Directory -Force -ErrorAction SilentlyContinue | - Where-Object { $_.Name -match '^[0-9]{8}-[0-9]{6}(-[0-9]+)?$' } | + Where-Object { $_.Name -match '^[0-9]{8}-[0-9]{6}(-[0-9]+)?$' -and (Test-DevSkillBackupMarker $_.FullName) } | Sort-Object -Property Name) $excess = $dirs.Count - $DevSkillBackupKeep foreach ($dir in $dirs) { @@ -346,6 +381,18 @@ function Backup-DevSkill([string]$path, [ref]$BackupPath) { if ($i -gt 1000) { throw "备份目录冲突无法解决,保留原目录: $path" } } New-Item -ItemType Directory -Path $backupRoot -Force -ErrorAction Stop | Out-Null + # Stamp ownership immediately after creating the stamp root and before + # any skill directory moves into it, so an interrupted backup can never + # leave an unmarked (never-prunable) stamp behind. + try { + Write-DevSkillBackupMarker $backupRoot + } catch { + # The removal stays non-recursive so a pre-existing non-empty root + # (foreign data) is never destroyed; a failed marker write must not + # leave an empty unowned stamp root behind either. + Remove-Item -LiteralPath $backupRoot -Force -ErrorAction SilentlyContinue + throw + } Move-DevSkillPathRecoverably $path $target if ($null -ne $BackupPath) { $BackupPath.Value = $target } try { diff --git a/scripts/install-devapp.sh b/scripts/install-devapp.sh index fe6c0d3f..d5716fd5 100755 --- a/scripts/install-devapp.sh +++ b/scripts/install-devapp.sh @@ -149,13 +149,23 @@ backup_skill_dir() { done record_current_run_backup_stamp "$backup_root" mkdir -p "$backup_root" || return 1 + # Ownership proof, the exact bytes Go's writeSkillBackupMarker stamps: a + # stamp-shaped name alone is not evidence, so pruning only ever removes + # roots carrying this marker — it must exist before the payload moves in. + printf '%s\n' 'dws skill backup v1' > "$backup_root/.dws-skill-backup" || { + rm -rf "$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 from earlier runs under $HOME/.dws/skill-backups, matching Go's -# skillBackupKeep / pruneSkillBackups. Stamp directories created by the +# skillBackupKeep / pruneSkillBackups. Only stamp-shaped roots carrying the +# .dws-skill-backup marker with the exact expected content are counted or +# removed: foreign data with a stamp-like name never consumes a keep slot +# and is never deleted, silently. Stamp directories created by the # current process are never pruned (mirroring Go's run-root registry and # install.js's currentRunBackupRoots), so a migration that retires more than # SKILL_BACKUP_KEEP batches stays reversible. Glob order is lexicographic and @@ -175,10 +185,12 @@ record_current_run_backup_stamp() { CURRENT_RUN_BACKUP_STAMPS="${CURRENT_RUN_BACKUP_STAMPS} $1" } -# is_skill_backup_stamp accepts only directory names DWS itself writes: UTC -# YYYYmmdd-HHMMSS, with an optional -N collision suffix. Any other entry in -# the backup root is foreign (user data, unrelated tooling) and is preserved -# so pruning can never remove a directory it cannot prove DWS created. +# is_skill_backup_stamp accepts only directory names with the stamp shape DWS +# itself writes: UTC YYYYmmdd-HHMMSS, with an optional -N collision suffix. +# Shape is necessary but not sufficient — pruning additionally verifies the +# .dws-skill-backup ownership marker — while any other entry in the backup +# root is foreign (user data, unrelated tooling) and is preserved so pruning +# can never remove a directory it cannot prove DWS created. is_skill_backup_stamp() { case "$1" in [0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9][0-9][0-9]) @@ -202,6 +214,7 @@ prune_skill_backups() { for prune_entry in "$prune_root"/*; do [ -d "$prune_entry" ] || continue is_skill_backup_stamp "${prune_entry##*/}" || continue + [ "$(cat "$prune_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue prune_total=$((prune_total + 1)) done [ "$prune_total" -gt "$SKILL_BACKUP_KEEP" ] || return 0 @@ -210,6 +223,7 @@ prune_skill_backups() { [ "$prune_drop" -gt 0 ] || break [ -d "$prune_entry" ] || continue is_skill_backup_stamp "${prune_entry##*/}" || continue + [ "$(cat "$prune_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue case " $CURRENT_RUN_BACKUP_STAMPS " in *" $prune_entry "*) continue ;; esac diff --git a/scripts/install-event.sh b/scripts/install-event.sh index 75933141..0665461d 100755 --- a/scripts/install-event.sh +++ b/scripts/install-event.sh @@ -218,13 +218,23 @@ backup_skill_dir() { done record_current_run_backup_stamp "$backup_root" mkdir -p "$backup_root" || return 1 + # Ownership proof, the exact bytes Go's writeSkillBackupMarker stamps: a + # stamp-shaped name alone is not evidence, so pruning only ever removes + # roots carrying this marker — it must exist before the payload moves in. + printf '%s\n' 'dws skill backup v1' > "$backup_root/.dws-skill-backup" || { + rm -rf "$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 from earlier runs under $HOME/.dws/skill-backups, matching Go's -# skillBackupKeep / pruneSkillBackups. Stamp directories created by the +# skillBackupKeep / pruneSkillBackups. Only stamp-shaped roots carrying the +# .dws-skill-backup marker with the exact expected content are counted or +# removed: foreign data with a stamp-like name never consumes a keep slot +# and is never deleted, silently. Stamp directories created by the # current process are never pruned (mirroring Go's run-root registry and # install.js's currentRunBackupRoots), so a migration that retires more than # SKILL_BACKUP_KEEP batches stays reversible. Glob order is lexicographic and @@ -244,10 +254,12 @@ record_current_run_backup_stamp() { CURRENT_RUN_BACKUP_STAMPS="${CURRENT_RUN_BACKUP_STAMPS} $1" } -# is_skill_backup_stamp accepts only directory names DWS itself writes: UTC -# YYYYmmdd-HHMMSS, with an optional -N collision suffix. Any other entry in -# the backup root is foreign (user data, unrelated tooling) and is preserved -# so pruning can never remove a directory it cannot prove DWS created. +# is_skill_backup_stamp accepts only directory names with the stamp shape DWS +# itself writes: UTC YYYYmmdd-HHMMSS, with an optional -N collision suffix. +# Shape is necessary but not sufficient — pruning additionally verifies the +# .dws-skill-backup ownership marker — while any other entry in the backup +# root is foreign (user data, unrelated tooling) and is preserved so pruning +# can never remove a directory it cannot prove DWS created. is_skill_backup_stamp() { case "$1" in [0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9][0-9][0-9]) @@ -271,6 +283,7 @@ prune_skill_backups() { for prune_entry in "$prune_root"/*; do [ -d "$prune_entry" ] || continue is_skill_backup_stamp "${prune_entry##*/}" || continue + [ "$(cat "$prune_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue prune_total=$((prune_total + 1)) done [ "$prune_total" -gt "$SKILL_BACKUP_KEEP" ] || return 0 @@ -279,6 +292,7 @@ prune_skill_backups() { [ "$prune_drop" -gt 0 ] || break [ -d "$prune_entry" ] || continue is_skill_backup_stamp "${prune_entry##*/}" || continue + [ "$(cat "$prune_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue case " $CURRENT_RUN_BACKUP_STAMPS " in *" $prune_entry "*) continue ;; esac diff --git a/scripts/install-skills.sh b/scripts/install-skills.sh index 6e67ec13..5ed3f101 100755 --- a/scripts/install-skills.sh +++ b/scripts/install-skills.sh @@ -67,11 +67,20 @@ backup_and_remove_skill_dir() { return 1 fi done - record_current_run_backup_stamp "$(dirname "$_bed_target")" - mkdir -p "$(dirname "$_bed_target")" 2>/dev/null || { + _bed_stamp_root="$(dirname "$_bed_target")" + record_current_run_backup_stamp "$_bed_stamp_root" + mkdir -p "$_bed_stamp_root" 2>/dev/null || { printf ' ⚠️ 无法创建备份目录,保留原目录 %s\n' "$_bed_dir" return 1 } + # Ownership proof, the exact bytes Go's writeSkillBackupMarker stamps: a + # stamp-shaped name alone is not evidence, so pruning only ever removes + # roots carrying this marker — it must exist before any payload moves in. + printf '%s\n' 'dws skill backup v1' > "$_bed_stamp_root/.dws-skill-backup" 2>/dev/null || { + rm -rf "$_bed_stamp_root" + printf ' ⚠️ 无法写入备份所有权标记,保留原目录 %s\n' "$_bed_dir" + return 1 + } if mv "$_bed_dir" "$_bed_target" 2>/dev/null; then DWS_LAST_SKILL_BACKUP="$_bed_target" printf ' × 已备份并移除 %s → %s\n' "$_bed_dir" "$_bed_target" @@ -83,7 +92,10 @@ backup_and_remove_skill_dir() { # prune_skill_backups keeps only the newest DWS_SKILL_BACKUP_KEEP stamped # backup directories from earlier runs under $HOME/.dws/skill-backups, -# matching Go's skillBackupKeep / pruneSkillBackups. Stamp directories created +# matching Go's skillBackupKeep / pruneSkillBackups. Only stamp-shaped roots +# carrying the .dws-skill-backup marker with the exact expected content are +# counted or removed: foreign data with a stamp-like name never consumes a +# keep slot and is never deleted, silently. Stamp directories created # by the current process are never pruned (mirroring Go's run-root registry # and install.js's currentRunBackupRoots), so a migration that retires more # than DWS_SKILL_BACKUP_KEEP batches stays reversible. Best-effort: a removal @@ -102,10 +114,12 @@ record_current_run_backup_stamp() { DWS_CURRENT_RUN_BACKUP_STAMPS="${DWS_CURRENT_RUN_BACKUP_STAMPS} $1" } -# is_skill_backup_stamp accepts only directory names DWS itself writes: UTC -# YYYYmmdd-HHMMSS, with an optional -N collision suffix. Any other entry in -# the backup root is foreign (user data, unrelated tooling) and is preserved -# so pruning can never remove a directory it cannot prove DWS created. +# is_skill_backup_stamp accepts only directory names with the stamp shape DWS +# itself writes: UTC YYYYmmdd-HHMMSS, with an optional -N collision suffix. +# Shape is necessary but not sufficient — pruning additionally verifies the +# .dws-skill-backup ownership marker — while any other entry in the backup +# root is foreign (user data, unrelated tooling) and is preserved so pruning +# can never remove a directory it cannot prove DWS created. is_skill_backup_stamp() { case "$1" in [0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9][0-9][0-9]) @@ -129,6 +143,7 @@ prune_skill_backups() { for _psb_entry in "$_psb_root"/*; do [ -d "$_psb_entry" ] || continue is_skill_backup_stamp "${_psb_entry##*/}" || continue + [ "$(cat "$_psb_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue _psb_total=$((_psb_total + 1)) done [ "$_psb_total" -gt "$DWS_SKILL_BACKUP_KEEP" ] || return 0 @@ -137,6 +152,7 @@ prune_skill_backups() { [ "$_psb_drop" -gt 0 ] || break [ -d "$_psb_entry" ] || continue is_skill_backup_stamp "${_psb_entry##*/}" || continue + [ "$(cat "$_psb_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue case " $DWS_CURRENT_RUN_BACKUP_STAMPS " in *" $_psb_entry "*) continue ;; esac diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 22d49438..23e0e652 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -765,6 +765,42 @@ function Get-SkillBackupName { $SkillBackupKeep = 5 $script:SkillBackupRootsThisRun = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) +# Ownership marker: every stamp root DWS creates carries .dws-skill-backup +# with exactly "dws skill backup v1" + LF — the same bytes internal/upgrade/ +# paths.go, install.sh, and build/npm/install.js write. A stamp-shaped name +# alone is not ownership proof, so pruning only deletes directories whose +# marker content verifies. +$SkillBackupMarkerFile = ".dws-skill-backup" +$SkillBackupMarkerBody = "dws skill backup v1" + +# Write-SkillBackupMarker stamps a freshly created stamp root as DWS-owned. +# [IO.File]::WriteAllText pins the exact LF-terminated bytes (Set-Content +# would append a platform newline on Windows PowerShell 5.1). +function Write-SkillBackupMarker { + param([string]$Root) + [System.IO.File]::WriteAllText( + (Join-Path $Root $SkillBackupMarkerFile), + "$SkillBackupMarkerBody`n", + [System.Text.UTF8Encoding]::new($false)) +} + +# Test-SkillBackupMarker reports whether a stamp root carries the ownership +# marker. The check normalizes CRLF→LF and drops trailing newlines before +# comparing, so this surface accepts every surface's exact-LF bytes (writer +# and checker agree); any other content, or a missing/unreadable marker, +# means foreign data. +function Test-SkillBackupMarker { + param([string]$Dir) + try { + $marker = Join-Path $Dir $SkillBackupMarkerFile + if (![System.IO.File]::Exists($marker)) { return $false } + $body = [System.IO.File]::ReadAllText($marker).Replace("`r`n", "`n").TrimEnd("`r", "`n") + return ($body -eq $SkillBackupMarkerBody) + } catch { + return $false + } +} + # 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 @@ -774,10 +810,11 @@ function Remove-OldSkillBackups { $root = Join-Path $HOME ".dws\skill-backups" if (!(Test-Path -LiteralPath $root -PathType Container)) { return } # Only directories whose names match the DWS backup stamp format (UTC - # yyyyMMdd-HHmmss, optional -N collision suffix) are candidates; any - # other entry is foreign data and is preserved. + # yyyyMMdd-HHmmss, optional -N collision suffix) AND whose ownership + # marker verifies are candidates; anything else is foreign data — + # preserved and never counted against $SkillBackupKeep. $dirs = @(Get-ChildItem -LiteralPath $root -Directory -Force -ErrorAction SilentlyContinue | - Where-Object { $_.Name -match '^[0-9]{8}-[0-9]{6}(-[0-9]+)?$' } | + Where-Object { $_.Name -match '^[0-9]{8}-[0-9]{6}(-[0-9]+)?$' -and (Test-SkillBackupMarker -Dir $_.FullName) } | Sort-Object -Property Name) $excess = $dirs.Count - $SkillBackupKeep foreach ($dir in $dirs) { @@ -817,6 +854,19 @@ function Backup-SkillDir { } try { New-Item -ItemType Directory -Path (Split-Path $target -Parent) -Force -ErrorAction Stop | Out-Null + # Stamp ownership immediately after creating the stamp root and + # before any skill directory moves into it, so an interrupted backup + # can never leave an unmarked (never-prunable) stamp behind. + Write-SkillBackupMarker -Root $targetRoot + } catch { + # The removal stays non-recursive so a pre-existing non-empty root + # (foreign data) is never destroyed; a failed marker write must not + # leave an empty unowned stamp root behind either. + Remove-Item -LiteralPath $targetRoot -Force -ErrorAction SilentlyContinue + Write-Say "⚠️ 备份失败,保留原目录 $Dir`: $_" + return $false + } + try { Move-SkillPathRecoverably -Source $Dir -Destination $target } catch { Write-Say "⚠️ 备份失败,保留原目录 $Dir`: $_" diff --git a/scripts/install.sh b/scripts/install.sh index aa511c5c..2beb6c93 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -85,11 +85,20 @@ backup_and_remove_skill_dir() { return 1 fi done - record_current_run_backup_stamp "$(dirname "$_bed_target")" - mkdir -p "$(dirname "$_bed_target")" 2>/dev/null || { + _bed_stamp_root="$(dirname "$_bed_target")" + record_current_run_backup_stamp "$_bed_stamp_root" + mkdir -p "$_bed_stamp_root" 2>/dev/null || { say " ⚠️ 无法创建备份目录,保留原目录 $_bed_dir" return 1 } + # Ownership proof, the exact bytes Go's writeSkillBackupMarker stamps: a + # stamp-shaped name alone is not evidence, so pruning only ever removes + # roots carrying this marker — it must exist before any payload moves in. + printf '%s\n' 'dws skill backup v1' > "$_bed_stamp_root/.dws-skill-backup" 2>/dev/null || { + rm -rf "$_bed_stamp_root" + say " ⚠️ 无法写入备份所有权标记,保留原目录 $_bed_dir" + return 1 + } if mv "$_bed_dir" "$_bed_target" 2>/dev/null; then DWS_LAST_SKILL_BACKUP="$_bed_target" say " × 已备份并移除 $_bed_dir → $_bed_target" @@ -101,7 +110,10 @@ backup_and_remove_skill_dir() { # prune_skill_backups keeps only the newest DWS_SKILL_BACKUP_KEEP stamped # backup directories from earlier runs under $HOME/.dws/skill-backups, -# matching Go's skillBackupKeep / pruneSkillBackups. Stamp directories created +# matching Go's skillBackupKeep / pruneSkillBackups. Only stamp-shaped roots +# carrying the .dws-skill-backup marker with the exact expected content are +# counted or removed: foreign data with a stamp-like name never consumes a +# keep slot and is never deleted, silently. Stamp directories created # by the current process are never pruned (mirroring Go's run-root registry # and install.js's currentRunBackupRoots), so a migration that retires more # than DWS_SKILL_BACKUP_KEEP batches stays reversible. Best-effort: a removal @@ -120,10 +132,12 @@ record_current_run_backup_stamp() { DWS_CURRENT_RUN_BACKUP_STAMPS="${DWS_CURRENT_RUN_BACKUP_STAMPS} $1" } -# is_skill_backup_stamp accepts only directory names DWS itself writes: UTC -# YYYYmmdd-HHMMSS, with an optional -N collision suffix. Any other entry in -# the backup root is foreign (user data, unrelated tooling) and is preserved -# so pruning can never remove a directory it cannot prove DWS created. +# is_skill_backup_stamp accepts only directory names with the stamp shape DWS +# itself writes: UTC YYYYmmdd-HHMMSS, with an optional -N collision suffix. +# Shape is necessary but not sufficient — pruning additionally verifies the +# .dws-skill-backup ownership marker — while any other entry in the backup +# root is foreign (user data, unrelated tooling) and is preserved so pruning +# can never remove a directory it cannot prove DWS created. is_skill_backup_stamp() { case "$1" in [0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9][0-9][0-9]) @@ -147,6 +161,7 @@ prune_skill_backups() { for _psb_entry in "$_psb_root"/*; do [ -d "$_psb_entry" ] || continue is_skill_backup_stamp "${_psb_entry##*/}" || continue + [ "$(cat "$_psb_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue _psb_total=$((_psb_total + 1)) done [ "$_psb_total" -gt "$DWS_SKILL_BACKUP_KEEP" ] || return 0 @@ -155,6 +170,7 @@ prune_skill_backups() { [ "$_psb_drop" -gt 0 ] || break [ -d "$_psb_entry" ] || continue is_skill_backup_stamp "${_psb_entry##*/}" || continue + [ "$(cat "$_psb_entry/.dws-skill-backup" 2>/dev/null)" = "dws skill backup v1" ] || continue case " $DWS_CURRENT_RUN_BACKUP_STAMPS " in *" $_psb_entry "*) continue ;; esac diff --git a/test/scripts/install_js_smoke.mjs b/test/scripts/install_js_smoke.mjs index da0cbc35..220b9320 100644 --- a/test/scripts/install_js_smoke.mjs +++ b/test/scripts/install_js_smoke.mjs @@ -53,6 +53,9 @@ * concurrently occupied destination (atomic * claim, no check-then-rename window) on * POSIX and simulated win32. + * 20. unmarked stamps — stamp-shaped directories without the exact + * ownership marker survive pruning and never + * consume a keep slot. * * Requirements: unix host with tar/zip/unzip on PATH (the same tools * install.js itself shells out to). Skips cleanly on win32. @@ -136,6 +139,18 @@ function writeFile(filePath, content, mode = 0o644) { fs.writeFileSync(filePath, content, { mode }); } +// Cross-surface backup ownership contract (mirrors internal/upgrade/paths.go, +// scripts/install.ps1, scripts/install-devapp.ps1, and scripts/install.sh): +// every DWS-created stamp root carries .dws-skill-backup whose entire content +// is exactly these bytes. Hardcoded here on purpose so drift from any writer +// fails the smoke test. +const SKILL_BACKUP_MARKER_FILE = ".dws-skill-backup"; +const SKILL_BACKUP_MARKER_BODY = "dws skill backup v1\n"; + +function markBackupStamp(stampRoot) { + writeFile(path.join(stampRoot, SKILL_BACKUP_MARKER_FILE), SKILL_BACKUP_MARKER_BODY); +} + function writeManagedState(home, names) { const state = { version: "old", @@ -1215,9 +1230,14 @@ scenario("simulated win32 publishes junctions with absolute canonical targets", 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", + fs.readdirSync(path.join(backupRoot, stamps[0])).sort(), + [SKILL_BACKUP_MARKER_FILE, ".zcode-skills-dingtalk-chat"], + "the stamp root carries the ownership marker next to the backup payload", + ); + assert.equal( + fs.readFileSync(path.join(backupRoot, stamps[0], SKILL_BACKUP_MARKER_FILE), "utf8"), + SKILL_BACKUP_MARKER_BODY, + "backup creation stamps the exact cross-surface marker bytes", ); assert.equal( fs.readFileSync(path.join(backupRoot, stamps[0], ".zcode-skills-dingtalk-chat", "SKILL.md"), "utf8"), @@ -1259,7 +1279,9 @@ scenario("skill backups stay bounded to the newest five stamps", () => { "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 stampRoot = path.join(backupRoot, stamp); + writeFile(path.join(stampRoot, "placeholder", "SKILL.md"), `${stamp}\n`); + markBackupStamp(stampRoot); } const victim = path.join(home, ".codex", "skills", "dingtalk-chat"); writeFile(path.join(victim, "SKILL.md"), "old codex copy\n"); @@ -1288,7 +1310,9 @@ scenario("prune preserves unknown directories in skill-backups", () => { "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 stampRoot = path.join(backupRoot, stamp); + writeFile(path.join(stampRoot, "placeholder", "SKILL.md"), `${stamp}\n`); + markBackupStamp(stampRoot); } const unknowns = [ "user-personal-backup", @@ -1322,6 +1346,73 @@ scenario("prune preserves unknown directories in skill-backups", () => { } }); +scenario("prune preserves unmarked stamp directories without consuming keep slots", () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "dws-installjs-prune-unmarked-")); + const home = path.join(tmp, "home"); + const backupRoot = path.join(home, ".dws", "skill-backups"); + try { + // Exactly SKILL_BACKUP_KEEP verified stamps: nothing may be pruned, even + // though foreign stamp-shaped siblings would push the naive count past + // the limit. + const marked = ["20260101-000001", "20260101-000002", "20260101-000003", "20260101-000004", "20260101-000005"]; + for (const stamp of marked) { + const stampRoot = path.join(backupRoot, stamp); + writeFile(path.join(stampRoot, "placeholder", "SKILL.md"), `${stamp}\n`); + markBackupStamp(stampRoot); + } + const foreign = [ + { stamp: "20260102-000001", body: null }, // no marker at all + { stamp: "20260102-000002", body: "dws skill backup v2\n" }, // wrong body + { stamp: "20260101-000000", body: "dws skill backup v1" }, // missing trailing LF + { stamp: "20260101-000099", body: "dws skill backup v1\r\n" }, // CRLF is not the exact contract bytes + ]; + for (const { stamp, body } of foreign) { + const stampRoot = path.join(backupRoot, stamp); + writeFile(path.join(stampRoot, "user-data.txt"), `foreign:${stamp}\n`); + if (body !== null) writeFile(path.join(stampRoot, SKILL_BACKUP_MARKER_FILE), body); + } + + pruneSkillBackups(home); + + let remaining = fs.readdirSync(backupRoot).sort(); + assert.deepEqual( + remaining, + [...marked, ...foreign.map(({ stamp }) => stamp)].sort(), + "with exactly keep-many verified stamps, nothing is pruned and unmarked stamps never count", + ); + + // Force real pruning: three more verified stamps push marked history to + // eight, so exactly the three oldest verified stamps go while every + // unmarked sibling stays put (still not consuming a keep slot). + for (const stamp of ["20260103-000001", "20260103-000002", "20260103-000003"]) { + const stampRoot = path.join(backupRoot, stamp); + writeFile(path.join(stampRoot, "placeholder", "SKILL.md"), `${stamp}\n`); + markBackupStamp(stampRoot); + } + + pruneSkillBackups(home); + + remaining = fs.readdirSync(backupRoot).sort(); + for (const pruned of marked.slice(0, 3)) { + assert.ok(!remaining.includes(pruned), `oldest verified stamp ${pruned} is pruned`); + } + assert.deepEqual( + remaining, + [marked[3], marked[4], ...foreign.map(({ stamp }) => stamp), "20260103-000001", "20260103-000002", "20260103-000003"].sort(), + "pruning keeps exactly the newest five verified stamps and preserves every unmarked sibling", + ); + for (const { stamp } of foreign) { + assert.equal( + fs.readFileSync(path.join(backupRoot, stamp, "user-data.txt"), "utf8"), + `foreign:${stamp}\n`, + `unmarked stamp ${stamp} contents must be intact`, + ); + } + } 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 06581f5d..300d2c0f 100644 --- a/test/scripts/install_script_test.go +++ b/test/scripts/install_script_test.go @@ -575,6 +575,209 @@ exit 0 } } +// The ownership-marker contract (skillBackupMarkerFile / skillBackupMarkerBody, +// declared in skill_backup_prune_test.go) is shared by the Go core +// (internal/upgrade/paths.go), install.sh, both PowerShell installers, and +// build/npm/install.js: every stamp root DWS creates carries the marker, and +// pruning only deletes stamp-named directories whose marker verifies. + +// seedSkillBackupOwnershipFixture fills ~/.dws/skill-backups with five +// verified stamps (marker bytes exactly as the Go core writes them, proving +// cross-surface acceptance) plus three foreign stamp-shaped siblings: two +// without a marker and one with a wrong marker body. +func seedSkillBackupOwnershipFixture(t *testing.T, home string) { + t.Helper() + backupRoot := filepath.Join(home, ".dws", "skill-backups") + for _, stamp := range []string{ + "20260101-000001", "20260101-000002", "20260101-000003", + "20260101-000004", "20260101-000005", + } { + mustWriteFile(t, filepath.Join(backupRoot, stamp, "placeholder", "SKILL.md"), []byte(stamp+"\n"), 0o644) + mustWriteFile(t, filepath.Join(backupRoot, stamp, skillBackupMarkerFile), []byte(skillBackupMarkerBody), 0o644) + } + for _, stamp := range []string{"20260102-000001", "20260102-000002", "20260101-000000"} { + mustWriteFile(t, filepath.Join(backupRoot, stamp, "user-data.txt"), []byte("foreign:"+stamp+"\n"), 0o644) + } + mustWriteFile(t, filepath.Join(backupRoot, "20260101-000000", skillBackupMarkerFile), []byte("dws skill backup v2\n"), 0o644) +} + +// assertSkillBackupOwnershipResult checks the post-prune contract shared by +// both PowerShell installers: the four oldest verified stamps are gone +// (three by the final prune, one by the backup's internal prune), exactly +// five verified stamps survive, every foreign stamp-shaped sibling is intact, +// and the current-run stamp carries the exact marker bytes next to the moved +// payload. +func assertSkillBackupOwnershipResult(t *testing.T, home string) { + t.Helper() + backupRoot := filepath.Join(home, ".dws", "skill-backups") + for _, pruned := range []string{"20260101-000001", "20260101-000002", "20260101-000003", "20260101-000004"} { + if _, err := os.Lstat(filepath.Join(backupRoot, pruned)); !os.IsNotExist(err) { + t.Fatalf("verified stamp %s must be pruned, err = %v", pruned, err) + } + } + entries, err := os.ReadDir(backupRoot) + if err != nil { + t.Fatalf("ReadDir(%s) error = %v", backupRoot, err) + } + surviving := map[string]bool{} + runStamps := []string{} + for _, entry := range entries { + surviving[entry.Name()] = true + if entry.Name() >= "20260801-000000" { + runStamps = append(runStamps, entry.Name()) + } + } + for _, want := range []string{ + "20260101-000005", + "20260101-000000", "20260102-000001", "20260102-000002", + "20260103-000001", "20260103-000002", "20260103-000003", + } { + if !surviving[want] { + t.Fatalf("stamp %s must survive pruning, surviving = %v", want, surviving) + } + } + if len(runStamps) != 1 || len(entries) != 8 { + t.Fatalf("expected the five newest verified stamps, three foreign stamps, and one run stamp, got %d entries (run stamps %v)", len(entries), runStamps) + } + runRoot := filepath.Join(backupRoot, runStamps[0]) + marker, err := os.ReadFile(filepath.Join(runRoot, skillBackupMarkerFile)) + if err != nil || string(marker) != skillBackupMarkerBody { + t.Fatalf("run stamp marker = %q, %v; want exactly %q", string(marker), err, skillBackupMarkerBody) + } + if backup := findSkillBackupContent(home, "old copy\n"); backup == "" { + t.Fatalf("backed-up Skill content missing from run stamp %s", runRoot) + } + for _, stamp := range []string{"20260101-000000", "20260102-000001", "20260102-000002"} { + data, err := os.ReadFile(filepath.Join(backupRoot, stamp, "user-data.txt")) + if err != nil || string(data) != "foreign:"+stamp+"\n" { + t.Fatalf("foreign stamp %s must be intact, got %q, %v", stamp, string(data), err) + } + } +} + +func TestInstallPowerShellPruneVerifiesBackupOwnershipMarker(t *testing.T) { + powerShellNames := []string{"pwsh"} + if runtime.GOOS == "windows" { + powerShellNames = []string{"powershell", "pwsh"} + } + pwsh := "" + for _, name := range powerShellNames { + if candidate, lookupErr := exec.LookPath(name); lookupErr == nil { + pwsh = candidate + break + } + } + if pwsh == "" { + t.Skip("PowerShell is not available") + } + + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", "install.ps1")) + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(scriptPath) + if err != nil { + t.Fatal(err) + } + cut := strings.LastIndex(string(data), "# ── Main") + if cut < 0 { + t.Fatal("install.ps1 main section not found") + } + prefix := strings.ReplaceAll(string(data[:cut]), "$HOME", "$env:DWS_TEST_HOME") + harness := prefix + ` +$backupRoot = Join-Path $env:DWS_TEST_HOME '.dws\skill-backups' +# Five verified stamps plus three foreign stamp-shaped siblings: the foreign +# entries must not count against the keep limit, so nothing is pruned here. +Remove-OldSkillBackups +foreach ($stamp in @('20260101-000001','20260101-000002','20260101-000003','20260101-000004','20260101-000005','20260101-000000','20260102-000001','20260102-000002')) { + if (!(Test-Path -LiteralPath (Join-Path $backupRoot $stamp) -PathType Container)) { exit 10 } +} +# A real backup both proves the writer stamps the exact marker bytes and adds +# a current-run root that pruning must keep. +$victim = Join-Path $env:DWS_TEST_HOME '.zcode\skills\dingtalk-chat' +New-Item -ItemType Directory -Path $victim -Force | Out-Null +[System.IO.File]::WriteAllText((Join-Path $victim 'SKILL.md'), "old copy` + "`n" + `") +if (!(Backup-SkillDir -Dir $victim)) { exit 11 } +if (Test-Path -LiteralPath $victim) { exit 12 } +foreach ($stamp in @('20260103-000001','20260103-000002','20260103-000003')) { + $stampRoot = Join-Path $backupRoot $stamp + New-Item -ItemType Directory -Path (Join-Path $stampRoot 'placeholder') -Force | Out-Null + Write-SkillBackupMarker -Root $stampRoot +} +Remove-OldSkillBackups +exit 0 +` + home := filepath.Join(t.TempDir(), "home") + seedSkillBackupOwnershipFixture(t, home) + harnessPath := filepath.Join(t.TempDir(), "skill-backup-marker.ps1") + mustWriteFile(t, harnessPath, []byte(harness), 0o644) + cmd := exec.Command(pwsh, "-NoProfile", "-NonInteractive", "-File", harnessPath) + cmd.Env = append(os.Environ(), "DWS_TEST_HOME="+home) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("PowerShell backup marker harness failed: %v\n%s", err, string(output)) + } + assertSkillBackupOwnershipResult(t, home) +} + +func TestInstallDevappPowerShellPruneVerifiesBackupOwnershipMarker(t *testing.T) { + pwsh := "" + if candidate, err := exec.LookPath("pwsh"); err == nil { + pwsh = candidate + } else if runtime.GOOS == "windows" { + if candidate, err := exec.LookPath("powershell"); err == nil { + pwsh = candidate + } + } + if pwsh == "" { + t.Skip("PowerShell is not available") + } + + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", "install-devapp.ps1")) + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(scriptPath) + if err != nil { + t.Fatal(err) + } + cut := strings.Index(string(data), "# Read the releases list") + if cut < 0 { + t.Fatal("install-devapp.ps1 release section not found") + } + prefix := strings.ReplaceAll(string(data[:cut]), "$HOME", "$env:DWS_TEST_HOME") + harness := prefix + ` +$backupRoot = Join-Path $env:DWS_TEST_HOME '.dws\skill-backups' +# Five verified stamps plus three foreign stamp-shaped siblings: the foreign +# entries must not count against the keep limit, so nothing is pruned here. +Remove-OldDevSkillBackups +foreach ($stamp in @('20260101-000001','20260101-000002','20260101-000003','20260101-000004','20260101-000005','20260101-000000','20260102-000001','20260102-000002')) { + if (!(Test-PathLexically (Join-Path $backupRoot $stamp))) { exit 10 } +} +$victim = Join-Path $env:DWS_TEST_HOME '.zcode\skills\dingtalk-chat' +New-Item -ItemType Directory -Path $victim -Force | Out-Null +[System.IO.File]::WriteAllText((Join-Path $victim 'SKILL.md'), "old copy` + "`n" + `") +Backup-DevSkill $victim +if (Test-PathLexically $victim) { exit 12 } +foreach ($stamp in @('20260103-000001','20260103-000002','20260103-000003')) { + $stampRoot = Join-Path $backupRoot $stamp + New-Item -ItemType Directory -Path (Join-Path $stampRoot 'placeholder') -Force | Out-Null + Write-DevSkillBackupMarker $stampRoot +} +Remove-OldDevSkillBackups +exit 0 +` + home := filepath.Join(t.TempDir(), "home") + seedSkillBackupOwnershipFixture(t, home) + harnessPath := filepath.Join(t.TempDir(), "devapp-skill-backup-marker.ps1") + mustWriteFile(t, harnessPath, []byte(harness), 0o644) + cmd := exec.Command(pwsh, "-NoProfile", "-NonInteractive", "-File", harnessPath) + cmd.Env = append(os.Environ(), "DWS_TEST_HOME="+home) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("PowerShell devapp backup marker harness failed: %v\n%s", err, string(output)) + } + assertSkillBackupOwnershipResult(t, home) +} + func TestInstallScriptLinkPublicationRacePreservesConcurrentObjects(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("POSIX shell semantics are unavailable") @@ -1007,6 +1210,15 @@ func TestInstallEventScriptInstallsBinaryAndEventSkills(t *testing.T) { if err != nil || string(sibling) != "keep sibling\n" { t.Fatalf("unrelated sibling changed: data=%q err=%v", sibling, err) } + // The replaced standalone Event skill must sit in a marker-stamped backup + // root: that marker is what lets a later prune count and recycle it. The + // event installer archives under the flattened source path, so locate the + // backup by content and the stamp root two levels above it. + if got := findSkillBackupContent(fakeHome, "old standalone event\n"); got == "" { + t.Fatalf("replaced standalone Event skill should be backed up under .dws/skill-backups") + } else { + assertSkillBackupMarker(t, filepath.Dir(filepath.Dir(got))) + } if _, err := os.Stat(filepath.Join(fakeHome, ".agents", "skills", "dingtalk-dev")); !os.IsNotExist(err) { t.Fatalf("dingtalk-dev should not be installed by install-event.sh, stat err=%v", err) } @@ -1571,12 +1783,17 @@ func TestInstallScriptSourceModeDefaultMultiInstall(t *testing.T) { } } // Removals must be reversible: the displaced dirs land under - // ~/.dws/skill-backups/ instead of being destroyed. - if findSkillBackup(fixture.fakeHome, "dws", "old mono\n") == "" { + // ~/.dws/skill-backups/ instead of being destroyed, each inside a stamp + // root stamped with the ownership marker. + if got := findSkillBackup(fixture.fakeHome, "dws", "old mono\n"); got == "" { t.Errorf("old mono dws/ should be backed up under .dws/skill-backups, output:\n%s", output) + } else { + assertSkillBackupMarker(t, filepath.Dir(got)) } - if findSkillBackup(fixture.fakeHome, "dingtalk-stale", "stale\n") == "" { + if got := findSkillBackup(fixture.fakeHome, "dingtalk-stale", "stale\n"); got == "" { t.Errorf("stale dingtalk-* should be backed up under .dws/skill-backups, output:\n%s", output) + } else { + assertSkillBackupMarker(t, filepath.Dir(got)) } if _, err := os.Stat(filepath.Join(base, "other-skill", "SKILL.md")); err != nil { t.Errorf("non-DWS skill must be preserved: %v", err) @@ -1738,6 +1955,73 @@ func findSkillBackup(fakeHome, name, content string) string { return found } +// assertSkillBackupMarker pins the ownership contract shared with the Go core: +// every stamp root a shell installer creates carries .dws-skill-backup with +// exactly "dws skill backup v1\n", because pruning only ever removes +// stamp-shaped roots with those exact bytes. +func assertSkillBackupMarker(t *testing.T, stampRoot string) { + t.Helper() + data, err := os.ReadFile(filepath.Join(stampRoot, skillBackupMarkerFile)) + if err != nil { + t.Fatalf("backup ownership marker missing in %s: %v", stampRoot, err) + } + if string(data) != skillBackupMarkerBody { + t.Fatalf("backup ownership marker in %s = %q, want %q", stampRoot, data, skillBackupMarkerBody) + } +} + +// TestInstallScriptPrunePreservesUnmarkedStampDirs runs the real install.sh +// (whose main ends with prune_skill_backups) against a HOME whose backup root +// already holds a full marked history plus one unmarked stamp-shaped +// directory. The end-of-run prune must remove the marked excess while the +// unmarked directory — foreign data without the ownership marker — is neither +// deleted nor counted toward the keep limit, and the stamp this run creates +// must carry the exact marker bytes. +func TestInstallScriptPrunePreservesUnmarkedStampDirs(t *testing.T) { + t.Parallel() + + fixture := newInstallSourceFixture(t) + installDir := filepath.Join(fixture.root, "bin") + backupRoot := filepath.Join(fixture.fakeHome, ".dws", "skill-backups") + + // Oldest on purpose: without the marker check this directory would be the + // first pruning candidate and would inflate the excess count. + unmarked := filepath.Join(backupRoot, "20200101-000000") + mustWriteFile(t, filepath.Join(unmarked, "user-data.txt"), []byte("foreign\n"), 0o644) + for i := 1; i <= 6; i++ { + mustWriteFile(t, filepath.Join(backupRoot, oldStampName(i), "skill", "SKILL.md"), []byte("old batch\n"), 0o644) + mustWriteFile(t, filepath.Join(backupRoot, oldStampName(i), skillBackupMarkerFile), []byte(skillBackupMarkerBody), 0o644) + } + seedAgentHome(t, fixture.fakeHome, "dws", "old mono\n") + + runInstallScript(t, fixture.scriptPath, fixture.envWithSkillMode("mono", + "DWS_INSTALL_DIR="+installDir, + "DWS_NO_SKILLS=0", + )) + + // Six marked batches plus this run's one exceed the keep limit of five, + // so exactly the two oldest marked batches are excess and must be gone — + // proving the prune actually ran. + for _, gone := range []string{oldStampName(1), oldStampName(2)} { + if _, err := os.Stat(filepath.Join(backupRoot, gone)); !os.IsNotExist(err) { + t.Fatalf("marked excess stamp %s should have been pruned, stat err=%v", gone, err) + } + } + if _, err := os.Stat(filepath.Join(backupRoot, oldStampName(3))); err != nil { + t.Fatalf("marked keep-range stamp should survive the prune: %v", err) + } + // The unmarked directory is never counted and never removed, even though + // it sorts before every pruned batch. + if data, err := os.ReadFile(filepath.Join(unmarked, "user-data.txt")); err != nil || string(data) != "foreign\n" { + t.Fatalf("unmarked stamp-shaped directory must be preserved untouched: data=%q err=%v", data, err) + } + got := findSkillBackup(fixture.fakeHome, "dws", "old mono\n") + if got == "" { + t.Fatal("this run's dws backup is missing after the prune") + } + assertSkillBackupMarker(t, filepath.Dir(got)) +} + // TestInstallScriptBackupFailureKeepsOriginalDir pins the fail-safe contract: // when the backup destination cannot be created (skill-backups pre-created as // a regular file), the installer must keep every pre-existing skill directory diff --git a/test/scripts/skill_backup_prune_test.go b/test/scripts/skill_backup_prune_test.go index 6defc75b..2e519490 100644 --- a/test/scripts/skill_backup_prune_test.go +++ b/test/scripts/skill_backup_prune_test.go @@ -34,6 +34,14 @@ var shellPruneScriptTargets = []string{ "install-devapp.sh", } +// The ownership-marker contract shared with the Go core +// (internal/upgrade/paths.go): every stamp root DWS creates carries the +// marker, and prune only counts or removes roots with these exact bytes. +const ( + skillBackupMarkerFile = ".dws-skill-backup" + skillBackupMarkerBody = "dws skill backup v1\n" +) + // extractShellFunction pulls a top-level `name() { ... }` definition out of a // script so the shipped logic — not a copy — is what the test exercises. func extractShellFunction(t *testing.T, scriptPath, name string) string { @@ -55,16 +63,33 @@ func extractShellFunction(t *testing.T, scriptPath, name string) string { return text[start : start+end+3] } +// mkdirStampWithMarker creates a DWS-owned stamp fixture: the stamp directory +// plus the exact ownership marker bytes prune verifies. Without the marker a +// stamp-shaped directory is foreign data that prune must neither count nor +// remove. +func mkdirStampWithMarker(t *testing.T, backupRoot, stamp string) { + t.Helper() + dir := filepath.Join(backupRoot, stamp) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + mustWriteFile(t, filepath.Join(dir, skillBackupMarkerFile), []byte(skillBackupMarkerBody), 0o644) +} + // runShellPruneScenario builds a sandbox HOME with the given pre-existing -// (foreign) stamps, registers the given stamps as created by the current run, -// runs the script's own prune_skill_backups, and returns the surviving stamp -// directories. -func runShellPruneScenario(t *testing.T, scriptPath string, foreignStamps, runStamps []string) []string { +// (earlier-run) stamps, registers the given stamps as created by the current +// run, optionally also plants stamp-shaped directories without the ownership +// marker, runs the script's own prune_skill_backups, and returns the +// surviving stamp directories. +func runShellPruneScenario(t *testing.T, scriptPath string, foreignStamps, runStamps []string, unmarkedStamps ...string) []string { t.Helper() home := t.TempDir() backupRoot := filepath.Join(home, ".dws", "skill-backups") for _, stamp := range foreignStamps { - if err := os.MkdirAll(filepath.Join(backupRoot, stamp), 0o755); err != nil { + mkdirStampWithMarker(t, backupRoot, stamp) + } + for _, stamp := range unmarkedStamps { + if err := os.MkdirAll(filepath.Join(backupRoot, stamp, "payload"), 0o755); err != nil { t.Fatal(err) } } @@ -83,9 +108,7 @@ func runShellPruneScenario(t *testing.T, scriptPath string, foreignStamps, runSt driver.WriteString(extractShellFunction(t, scriptPath, "prune_skill_backups")) driver.WriteString("\n") for _, stamp := range runStamps { - if err := os.MkdirAll(filepath.Join(backupRoot, stamp), 0o755); err != nil { - t.Fatal(err) - } + mkdirStampWithMarker(t, backupRoot, stamp) driver.WriteString("record_current_run_backup_stamp \"$HOME/.dws/skill-backups/" + stamp + "\"\n") } driver.WriteString("prune_skill_backups\n") @@ -179,6 +202,24 @@ func TestInstallShellPruneKeepsCurrentRunBackups(t *testing.T) { oldStampName(3), oldStampName(4), oldStampName(5), oldStampName(6), oldStampName(7)) }) + + // A stamp-shaped directory without the ownership marker is foreign + // data: it must be preserved untouched, never counted toward the + // keep limit, and never treated as prunable excess. Oldest on + // purpose — if the marker were not required, this directory would + // be the first pruning candidate. + t.Run("unmarked stamp-shaped directory is preserved and uncounted", func(t *testing.T) { + var foreignStamps []string + for i := 1; i <= 7; i++ { + foreignStamps = append(foreignStamps, oldStampName(i)) + } + unmarked := "20200101-000000" + surviving := runShellPruneScenario(t, scriptPath, foreignStamps, nil, unmarked) + assertStampsSurvive(t, surviving, + append([]string{unmarked}, + oldStampName(3), oldStampName(4), oldStampName(5), + oldStampName(6), oldStampName(7))...) + }) }) } }