diff --git a/scripts/install-devapp.ps1 b/scripts/install-devapp.ps1 index ce88a4a3..1719391d 100644 --- a/scripts/install-devapp.ps1 +++ b/scripts/install-devapp.ps1 @@ -110,7 +110,13 @@ function Test-PathLexically([string]$path) { } function Move-DevSkillPath([string]$Source, [string]$Destination) { - Move-Item -LiteralPath $Source -Destination $Destination -ErrorAction Stop + if (Test-PathLexically $Destination) { throw "Skill move destination already exists: $Destination" } + $sourceItem = Get-Item -LiteralPath $Source -Force -ErrorAction Stop + if ($sourceItem.PSIsContainer) { + [System.IO.Directory]::Move($Source, $Destination) + } else { + [System.IO.File]::Move($Source, $Destination) + } } function Test-DevCrossDeviceMoveError([System.Management.Automation.ErrorRecord]$Record) { @@ -263,8 +269,60 @@ function Backup-DevSkill([string]$path, [ref]$BackupPath) { if ($null -ne $BackupPath) { $BackupPath.Value = $target } } -function Restore-DevSkillBackup([string]$Destination, [string]$Backup) { - if (Test-PathLexically $Destination) { Remove-DevSkillPathLexically $Destination } +function Get-DevSkillLinkSignature([string]$Path) { + try { $item = Get-Item -LiteralPath $Path -Force -ErrorAction Stop } catch { return $null } + if (!$item.LinkType) { return $null } + return ([string]$item.LinkType + "`0" + ((@($item.Target) | ForEach-Object { [string]$_ }) -join "`0")) +} + +function New-DevRollbackFailure([string]$Message) { + $failure = [System.InvalidOperationException]::new($Message) + $failure.Data["DWSRollbackFailed"] = $true + return $failure +} + +function Remove-VerifiedDevSkillPublication( + [string]$Destination, + [string]$PublishedSource, + [string]$PublishedLinkSignature +) { + $parent = Split-Path $Destination -Parent + $quarantine = Join-Path $parent (".dws-dev-rollback-" + [guid]::NewGuid().ToString("N")) + Move-DevSkillPath $Destination $quarantine + try { + if (![string]::IsNullOrWhiteSpace($PublishedLinkSignature)) { + if ((Get-DevSkillLinkSignature $quarantine) -ne $PublishedLinkSignature) { + throw "published Skill link was replaced by another process: $Destination" + } + } elseif (![string]::IsNullOrWhiteSpace($PublishedSource)) { + Assert-DevSkillPathCopy $PublishedSource $quarantine + } else { + throw "Skill restore destination is occupied by another process: $Destination" + } + Remove-DevSkillPathLexically $quarantine + } catch { + $failure = $_ + if (Test-PathLexically $quarantine) { + try { + if (Test-PathLexically $Destination) { throw "original path is occupied" } + Move-DevSkillPath $quarantine $Destination + } catch { + throw "$failure; concurrent object retained at $quarantine`: $_" + } + } + throw $failure + } +} + +function Restore-DevSkillBackup( + [string]$Destination, + [string]$Backup, + [string]$PublishedSource = "", + [string]$PublishedLinkSignature = "" +) { + if (Test-PathLexically $Destination) { + Remove-VerifiedDevSkillPublication $Destination $PublishedSource $PublishedLinkSignature + } if (![string]::IsNullOrWhiteSpace($Backup)) { Move-DevSkillPathRecoverably $Backup $Destination } @@ -276,6 +334,7 @@ function Publish-DevSkillCopy([string]$Source, [string]$Destination) { $stageRoot = Join-Path $parent (".dws-dev-copy-" + [guid]::NewGuid().ToString("N")) $stage = Join-Path $stageRoot "payload" $backup = "" + $publishedSource = "" New-Item -ItemType Directory -Path $stageRoot -ErrorAction Stop | Out-Null try { Copy-DevSkillPathLexically $Source $stage @@ -283,11 +342,12 @@ function Publish-DevSkillCopy([string]$Source, [string]$Destination) { Backup-DevSkill $Destination ([ref]$backup) try { Move-DevSkillPath $stage $Destination + $publishedSource = $Source Assert-DevSkillPathCopy $Source $Destination } catch { $publishFailure = $_ - try { Restore-DevSkillBackup $Destination $backup } catch { - throw "$publishFailure; Skill rollback failed; backup retained at $backup`: $_" + try { Restore-DevSkillBackup $Destination $backup $publishedSource "" } catch { + throw (New-DevRollbackFailure "$publishFailure; Skill rollback failed; backup retained at $backup`: $_") } throw $publishFailure } @@ -302,6 +362,7 @@ function Publish-DevSkillJunction([string]$Target, [string]$Destination) { $stageRoot = Join-Path $parent (".dws-dev-link-" + [guid]::NewGuid().ToString("N")) $stage = Join-Path $stageRoot "payload" $backup = "" + $publishedSignature = "" New-Item -ItemType Directory -Path $stageRoot -ErrorAction Stop | Out-Null try { New-Item -ItemType Junction -Path $stage -Target ([System.IO.Path]::GetFullPath($Target)) -ErrorAction Stop | Out-Null @@ -310,10 +371,12 @@ function Publish-DevSkillJunction([string]$Target, [string]$Destination) { Move-DevSkillPath $stage $Destination $published = Get-Item -LiteralPath $Destination -Force -ErrorAction Stop if (!$published.LinkType) { throw "published Skill path is not a junction: $Destination" } + $publishedSignature = Get-DevSkillLinkSignature $Destination + if ([string]::IsNullOrEmpty($publishedSignature)) { throw "published Skill link identity is unavailable: $Destination" } } catch { $publishFailure = $_ - try { Restore-DevSkillBackup $Destination $backup } catch { - throw "$publishFailure; Skill rollback failed; backup retained at $backup`: $_" + try { Restore-DevSkillBackup $Destination $backup "" $publishedSignature } catch { + throw (New-DevRollbackFailure "$publishFailure; Skill rollback failed; backup retained at $backup`: $_") } throw $publishFailure } @@ -440,16 +503,22 @@ if (-not $NoSkills) { } continue } - New-Item -ItemType Directory -Path $base -Force | Out-Null # Per-agent degrade like install.sh: a failed link→copy fallback # skips that agent loudly instead of aborting the whole loop. - try { Publish-DevSkillJunction $canonical $dest } - catch { - try { Publish-DevSkillCopy $canonical $dest } catch { - Say "⚠️ Agent Skill 目标安装失败,已跳过: $dest ($($_.Exception.Message))" - $failed++ - continue + try { + New-Item -ItemType Directory -Path $base -Force -ErrorAction Stop | Out-Null + try { Publish-DevSkillJunction $canonical $dest } + catch { + # A failed rollback leaves the old backup as the only + # trusted copy. Do not run a second publisher over that + # state; surface the failure and continue other agents. + if ($_.Exception.Data["DWSRollbackFailed"]) { throw } + Publish-DevSkillCopy $canonical $dest } + } catch { + Say "⚠️ Agent Skill 目标安装失败,已跳过: $dest ($($_.Exception.Message))" + $failed++ + continue } $installed++ } diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 50540929..4cda9a7d 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -258,7 +258,19 @@ function Move-SkillPath { [string]$Source, [string]$Destination ) - Move-Item -LiteralPath $Source -Destination $Destination -ErrorAction Stop + if (Test-SkillPathLexically -Path $Destination) { + throw "Skill move destination already exists: $Destination" + } + $sourceItem = Get-Item -LiteralPath $Source -Force -ErrorAction Stop + if ($sourceItem.PSIsContainer) { + # Move-Item treats an existing directory as a container and nests the + # source below it. Directory.Move has the exact rename semantics this + # transaction requires: an occupied destination fails without touching + # either path. + [System.IO.Directory]::Move($Source, $Destination) + } else { + [System.IO.File]::Move($Source, $Destination) + } } function Test-CrossDeviceMoveError { @@ -408,10 +420,16 @@ function Get-PhysicalSkillPath { try { $item = Get-Item -LiteralPath $candidate -Force -ErrorAction Stop } catch { return $null } $target = @($item.Target) | Where-Object { $_ } | Select-Object -First 1 if ($item.LinkType -and $target) { + $targetPath = "" if ([System.IO.Path]::IsPathRooted([string]$target)) { - return ([string]$target).TrimEnd([char[]]@('\', '/')) + $targetPath = [string]$target + } else { + $targetPath = Join-Path $parentPhysical ([string]$target) } - return [System.IO.Path]::GetFullPath((Join-Path $parentPhysical ([string]$target))) + # Resolve the target recursively as well. A custom HOME or canonical + # root can itself sit below another junction, which must compare equal + # to the fully physical path just like EvalSymlinks/realpath do. + return Get-PhysicalSkillPath -Path ([System.IO.Path]::GetFullPath($targetPath)) -Depth ($Depth + 1) } return $candidate } @@ -723,6 +741,68 @@ function Backup-SkillDir { return $true } +function Get-SkillLinkSignature { + param([string]$Path) + try { + $item = Get-Item -LiteralPath $Path -Force -ErrorAction Stop + } catch { + return $null + } + if (!$item.LinkType) { return $null } + $targets = @($item.Target) | ForEach-Object { [string]$_ } + return ([string]$item.LinkType + "`0" + ($targets -join "`0")) +} + +function New-PublishedSkillLinkRecord { + param([string]$Path) + $signature = Get-SkillLinkSignature -Path $Path + if ([string]::IsNullOrEmpty($signature)) { + throw "published Skill path is not a link: $Path" + } + return [pscustomobject]@{ Path = $Path; LinkSignature = $signature } +} + +function New-PublishedSkillCopyRecord { + param([string]$Path, [string]$Source) + Assert-SkillPathCopy -Source $Source -Destination $Path + return [pscustomobject]@{ Path = $Path; Source = $Source } +} + +function Remove-PublishedSkillPathSafely { + param($Record) + $path = [string]$Record.Path + $parent = Split-Path $path -Parent + $quarantine = Join-Path $parent (".dws-link-rollback-" + [guid]::NewGuid().ToString("N")) + + # Claim the current directory entry with an exact same-filesystem rename. + # If another process replaced our link, verification below fails and that + # object is moved back instead of ever being recursively removed. + Move-SkillPath -Source $path -Destination $quarantine + try { + if ($Record.LinkSignature) { + if ((Get-SkillLinkSignature -Path $quarantine) -ne [string]$Record.LinkSignature) { + throw "发布链接已被其他进程替换,拒绝删除" + } + } elseif ($Record.Source) { + Assert-SkillPathCopy -Source ([string]$Record.Source) -Destination $quarantine + } else { + throw "发布路径缺少事务身份,拒绝删除" + } + Remove-SkillPathLexically -Path $quarantine + } catch { + $failure = $_ + if (Test-SkillPathLexically -Path $quarantine) { + try { + if (Test-SkillPathLexically -Path $path) { throw "原路径已被占用" } + Move-SkillPath -Source $quarantine -Destination $path + } catch { + throw "$failure;并发对象保留于 $quarantine`: $_" + } + } + throw $failure + } +} + function Restore-MultiSkillSet { param( [array]$Published, @@ -730,16 +810,18 @@ function Restore-MultiSkillSet { ) $ok = $true for ($i = $Published.Count - 1; $i -ge 0; $i--) { + $publishedItem = $Published[$i] + $publishedPath = if ($publishedItem -is [string]) { $publishedItem } else { [string]$publishedItem.Path } try { - if (Test-SkillPathLexically -Path $Published[$i]) { - # Junction publications must never be removed with -Recurse: - # Windows PowerShell 5.1 can follow the reparse point and - # delete the canonical store's contents. Link targets remove - # lexically; copied directories still remove recursively. - Remove-SkillPathLexically -Path $Published[$i] + if (Test-SkillPathLexically -Path $publishedPath) { + if (!($publishedItem -is [string])) { + Remove-PublishedSkillPathSafely -Record $publishedItem + } else { + Remove-SkillPathLexically -Path $publishedPath + } } } catch { - Write-Say "⚠️ 无法移除失败发布目录 $($Published[$i]): $_" + Write-Say "⚠️ 无法移除失败发布目录 $publishedPath`: $_" $ok = $false } } @@ -899,8 +981,11 @@ function Publish-CanonicalSkillLinks { } foreach ($name in $publishNames) { $dest = Join-Path $BaseDir $name - $published += $dest Move-SkillPath -Source (Join-Path $stageRoot $name) -Destination $dest + # Only record a publication after the staged junction has occupied + # the exact destination. Rollback also checks this identity before + # deleting, so a concurrent user directory is never removed. + $published += New-PublishedSkillLinkRecord -Path $dest Write-Say "↪ Skills → $dest" } return $true @@ -1188,8 +1273,8 @@ function Install-MonoToBase { } } - $published += $dest Move-SkillPath -Source $stagedSkill -Destination $dest + $published += New-PublishedSkillCopyRecord -Path $dest -Source $SkillSrc } catch { $transactionError = $_ if (!(Restore-MultiSkillSet -Published $published -Backups $backups)) { @@ -1383,8 +1468,8 @@ function Install-MultiToBase { foreach ($skillDir in $skillDirs) { $dest = Join-Path $BaseDir $skillDir.Name - $published += $dest Move-SkillPath -Source (Join-Path $stageRoot $skillDir.Name) -Destination $dest + $published += New-PublishedSkillCopyRecord -Path $dest -Source $skillDir.FullName } } catch { $transactionError = $_ diff --git a/test/scripts/install_script_test.go b/test/scripts/install_script_test.go index d69846ad..006cabc2 100644 --- a/test/scripts/install_script_test.go +++ b/test/scripts/install_script_test.go @@ -417,12 +417,18 @@ New-Item -ItemType Junction -Path $junction -Target $canonical -ErrorAction Sile if (Test-Path -LiteralPath $junction) { $asserted = 1 if (!(Test-SamePhysicalSkillRoot -Left $junction -Right $canonical)) { exit 3 } + $junctionChain = Join-Path $linkParent 'dingtalk-chat-chain' + New-Item -ItemType Junction -Path $junctionChain -Target $junction -ErrorAction Stop | Out-Null + if (!(Test-SamePhysicalSkillRoot -Left $junctionChain -Right $canonical)) { exit 5 } } $symlink = Join-Path $linkParent 'dingtalk-shared' New-Item -ItemType SymbolicLink -Path $symlink -Target $other -ErrorAction SilentlyContinue | Out-Null if (Test-Path -LiteralPath $symlink) { $asserted = 1 if (!(Test-SamePhysicalSkillRoot -Left $symlink -Right $other)) { exit 3 } + $symlinkChain = Join-Path $linkParent 'dingtalk-shared-chain' + New-Item -ItemType SymbolicLink -Path $symlinkChain -Target $symlink -ErrorAction Stop | Out-Null + if (!(Test-SamePhysicalSkillRoot -Left $symlinkChain -Right $other)) { exit 5 } } if ($asserted -eq 0) { exit 4 } if (Test-SamePhysicalSkillRoot -Left $canonical -Right $other) { exit 3 } @@ -544,6 +550,20 @@ Set-Content -LiteralPath (Join-Path $publishedPath 'concurrent-user-data.txt') - $restored = Restore-MultiSkillSet -Published @($record) -Backups @() if ($restored) { exit 15 } if (!(Test-Path -LiteralPath (Join-Path $publishedPath 'concurrent-user-data.txt') -PathType Leaf)) { exit 16 } + +# Copied mono/multi publications use the same identity-protected rollback. +$copySource = Join-Path $root 'copy-source' +$copyPath = Join-Path $root 'published-copy' +New-Item -ItemType Directory -Path $copySource -Force | Out-Null +Set-Content -LiteralPath (Join-Path $copySource 'SKILL.md') -Value 'transaction copy' +Copy-SkillPathLexically -Source $copySource -Destination $copyPath +$copyRecord = New-PublishedSkillCopyRecord -Path $copyPath -Source $copySource +Remove-SkillPathLexically -Path $copyPath +New-Item -ItemType Directory -Path $copyPath -Force | Out-Null +Set-Content -LiteralPath (Join-Path $copyPath 'concurrent-copy-data.txt') -Value 'keep' +$copyRestored = Restore-MultiSkillSet -Published @($copyRecord) -Backups @() +if ($copyRestored) { exit 17 } +if (!(Test-Path -LiteralPath (Join-Path $copyPath 'concurrent-copy-data.txt') -PathType Leaf)) { exit 18 } exit 0 ` harnessPath := filepath.Join(t.TempDir(), "publication-race.ps1") @@ -3050,6 +3070,80 @@ try { } } +func TestInstallDevappPowerShellPublicationRacePreservesConcurrentDirectory(t *testing.T) { + powerShellNames := []string{"pwsh"} + if runtime.GOOS == "windows" { + powerShellNames = []string{"powershell", "pwsh"} + } + pwsh := "" + for _, name := range powerShellNames { + if candidate, err := exec.LookPath(name); err == nil { + pwsh = candidate + break + } + } + 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") + } + root := t.TempDir() + home := filepath.Join(root, "home") + source := filepath.Join(root, "bundle", "dingtalk-misc") + destination := filepath.Join(root, "external", "dingtalk-misc") + mustWriteFile(t, filepath.Join(source, "SKILL.md"), []byte("new dev skill\n"), 0o640) + mustWriteFile(t, filepath.Join(destination, "SKILL.md"), []byte("old dev skill\n"), 0o644) + + prefix := strings.ReplaceAll(string(data[:cut]), "$HOME", "$env:DWS_TEST_HOME") + harness := prefix + ` +$script:OriginalBackupDevSkill = ${function:Backup-DevSkill} +function Backup-DevSkill([string]$path, [ref]$BackupPath) { + & $script:OriginalBackupDevSkill $path $BackupPath + # Simulate another installer recreating the destination after the old + # content was backed up but before this transaction publishes its stage. + New-Item -ItemType Directory -Path $path -Force -ErrorAction Stop | Out-Null + Set-Content -LiteralPath (Join-Path $path 'concurrent-user-data.txt') -Value 'keep' +} +try { + Publish-DevSkillCopy $env:DWS_TEST_SOURCE $env:DWS_TEST_DESTINATION + exit 2 +} catch { + if (!$_.Exception.Data['DWSRollbackFailed']) { Write-Error $_; exit 3 } +} +if (!(Test-Path -LiteralPath (Join-Path $env:DWS_TEST_DESTINATION 'concurrent-user-data.txt') -PathType Leaf)) { exit 4 } +if (Test-Path -LiteralPath (Join-Path $env:DWS_TEST_DESTINATION 'payload')) { exit 5 } +exit 0 +` + harnessPath := filepath.Join(t.TempDir(), "install-devapp-publication-race.ps1") + mustWriteFile(t, harnessPath, []byte(harness), 0o644) + cmd := exec.Command(pwsh, "-NoProfile", "-NonInteractive", "-File", harnessPath) + cmd.Env = append(os.Environ(), + "DWS_TEST_HOME="+home, + "DWS_TEST_SOURCE="+source, + "DWS_TEST_DESTINATION="+destination, + ) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("PowerShell devapp publication race must retain concurrent directory: %v\n%s", err, output) + } + if got, err := os.ReadFile(filepath.Join(destination, "concurrent-user-data.txt")); err != nil || strings.TrimSpace(string(got)) != "keep" { + t.Fatalf("concurrent devapp data = %q, %v", got, err) + } + if backup := findSkillBackup(home, "dingtalk-misc", "old dev skill\n"); backup == "" { + t.Fatal("original devapp backup must be retained after publication race") + } +} + func TestInstallPowerShellMonoTransactionFailuresRestoreOldSet(t *testing.T) { pwsh, err := exec.LookPath("pwsh") if err != nil {