diff --git a/Pull-SDLC.ai.Tests.ps1 b/Pull-SDLC.ai.Tests.ps1 index bf52358..899dc35 100644 --- a/Pull-SDLC.ai.Tests.ps1 +++ b/Pull-SDLC.ai.Tests.ps1 @@ -1360,6 +1360,54 @@ Describe 'Invoke-PullSDLC auto-worktree mode' { (Get-Content (Join-Path $wt 'CLAUDE.md') -Raw) | Should -Be 'b' } + It 'recovers from a stale worktree marker pointing to a foreign / deleted gitdir (issue #180)' { + # Reproduces the real PSBitWarden symptom: a leftover .worktrees/sdlc-sync/ + # directory whose .git file says `gitdir: ` (or a + # deleted path). The reuse check that only inspects the marker's + # existence is fooled, then every git command inside the directory + # fails with "fatal: not a git repository: (NULL)" and the auto- + # worktree flow blows up. The fix discards the stale directory and + # creates a fresh worktree. + $fx = New-DiffReplayFixture -Root $script:fixtureRoot ` + -Seed { 'a' | Out-File -Encoding utf8 CLAUDE.md -NoNewline } ` + -Tweak { 'b' | Out-File -Encoding utf8 CLAUDE.md -NoNewline } + $origin = Join-Path (Split-Path $fx.Consumer -Parent) 'origin.git' + git init --bare -q -b main $origin + git -C $fx.Consumer remote remove origin 2>$null | Out-Null + git -C $fx.Consumer remote add origin $origin + Push-Location $fx.Consumer + try { + git checkout -q main + git push -q origin main + } finally { Pop-Location } + # Manually fabricate a stale worktree directory: a directory with a + # .git FILE whose gitdir points somewhere this repo knows nothing about. + $wt = Join-Path $fx.Consumer '.worktrees/sdlc-sync' + New-Item -ItemType Directory -Path $wt -Force | Out-Null + $foreignGitDir = Join-Path (Split-Path $fx.Consumer -Parent) 'foreign-repo/.git/worktrees/sdlc-sync' + Set-Content -LiteralPath (Join-Path $wt '.git') -Value ("gitdir: $foreignGitDir") -NoNewline + 'leftover from a different repo' | Out-File -Encoding utf8 (Join-Path $wt 'STRAY.md') -NoNewline + + $rc = Invoke-PullSDLC -RepoRoot $fx.Consumer -RemoteName 'sdlc.ai' -Bootstrap -NoFetch -NoAutoPR -CommitOnMain:$false + $rc | Should -Be 0 + # The worktree must now belong to this repo and carry the synced content. + Test-Path (Join-Path $wt '.git') | Should -BeTrue + Push-Location $wt + try { + $wtCommonDir = (git rev-parse --git-common-dir).Trim() + $wtAbs = (Resolve-Path -LiteralPath $wtCommonDir).Path + } finally { Pop-Location } + Push-Location $fx.Consumer + try { + $thisCommonDir = (git rev-parse --git-common-dir).Trim() + $thisAbs = (Resolve-Path -LiteralPath $thisCommonDir).Path + } finally { Pop-Location } + $wtAbs | Should -Be $thisAbs + # Stray content from the old foreign tree is gone; sync produced new content. + Test-Path (Join-Path $wt 'STRAY.md') | Should -BeFalse + (Get-Content (Join-Path $wt 'CLAUDE.md') -Raw) | Should -Be 'b' + } + It 'auto-recovers a divergent push to chore/sdlc-sync via force-with-lease retry' { # Reproduces the real-world divergent-scratch-branch case: a previous # successful run pushed commit X to origin/chore/sdlc-sync. The @@ -1471,36 +1519,43 @@ Describe 'Invoke-SelfRefresh' { Set-Content -LiteralPath $script:scriptPath -Value 'original-body' -NoNewline } - It 'returns $false and leaves the file unchanged when remote hash matches local' { + It 'returns empty and leaves the file unchanged when remote hash matches local' { Mock -CommandName Invoke-WebRequest -MockWith { param($Uri, $OutFile, $TimeoutSec, $UseBasicParsing) Set-Content -LiteralPath $OutFile -Value 'original-body' -NoNewline } $result = Invoke-SelfRefresh -ScriptPath $script:scriptPath - $result | Should -BeFalse + $result | Should -BeNullOrEmpty (Get-Content -LiteralPath $script:scriptPath -Raw) | Should -Be 'original-body' } - It 'returns $true and overwrites the local file when remote hash differs' { + It 'returns a temp file path and does NOT overwrite the local file when remote hash differs (issue #180)' { Mock -CommandName Invoke-WebRequest -MockWith { param($Uri, $OutFile, $TimeoutSec, $UseBasicParsing) Set-Content -LiteralPath $OutFile -Value 'NEW-upstream-body' -NoNewline } $result = Invoke-SelfRefresh -ScriptPath $script:scriptPath - $result | Should -BeTrue - (Get-Content -LiteralPath $script:scriptPath -Raw) | Should -Be 'NEW-upstream-body' + $result | Should -Not -BeNullOrEmpty + $result | Should -Match 'pull-sdlc-self-.*\.ps1$' + Test-Path -LiteralPath $result | Should -BeTrue + # The on-disk script must NOT be overwritten (regression for issue #180). + (Get-Content -LiteralPath $script:scriptPath -Raw) | Should -Be 'original-body' + # The returned temp file holds the new upstream content. + (Get-Content -LiteralPath $result -Raw) | Should -Be 'NEW-upstream-body' + # Clean up the temp file the function intentionally leaves behind for the caller. + Remove-Item -LiteralPath $result -Force -ErrorAction SilentlyContinue } - It 'returns $false and emits a warning when Invoke-WebRequest throws' { + It 'returns empty and emits a warning when Invoke-WebRequest throws' { Mock -CommandName Invoke-WebRequest -MockWith { throw 'simulated network failure' } $warnings = @() $result = Invoke-SelfRefresh -ScriptPath $script:scriptPath -WarningVariable warnings -WarningAction SilentlyContinue - $result | Should -BeFalse + $result | Should -BeNullOrEmpty (Get-Content -LiteralPath $script:scriptPath -Raw) | Should -Be 'original-body' ($warnings -join ' ') | Should -Match 'Self-update check skipped' } - It 'deletes its temp file even when $WhatIfPreference is true' { + It 'deletes its temp file even when $WhatIfPreference is true (no-update path)' { # Clean any stragglers from prior runs so this assertion is hermetic. Get-ChildItem -Path ([System.IO.Path]::GetTempPath()) -Filter 'pull-sdlc-self-*.ps1' -ErrorAction SilentlyContinue | Remove-Item -Force -ErrorAction SilentlyContinue @@ -1517,7 +1572,7 @@ Describe 'Invoke-SelfRefresh' { finally { $WhatIfPreference = $false } - $result | Should -BeFalse + $result | Should -BeNullOrEmpty $leftover = Get-ChildItem -Path ([System.IO.Path]::GetTempPath()) -Filter 'pull-sdlc-self-*.ps1' -ErrorAction SilentlyContinue $leftover | Should -BeNullOrEmpty } @@ -1572,9 +1627,14 @@ Describe 'Invoke-SelfRefresh' { param($Uri, $OutFile, $TimeoutSec, $UseBasicParsing, $Headers) Set-Content -LiteralPath $OutFile -Value 'NEW-upstream-body' -NoNewline -WhatIf:$false } + $tempPath = & { Invoke-SelfRefresh -ScriptPath $script:scriptPath -Verbose 4>&1 } | + Where-Object { $_ -is [string] } | Select-Object -First 1 $verboseMsgs = & { Invoke-SelfRefresh -ScriptPath $script:scriptPath -Verbose 4>&1 } | Where-Object { $_ -is [System.Management.Automation.VerboseRecord] } ($verboseMsgs -join ' ') | Should -Match 'Self-refresh: updated' + # Clean up temp files produced by both invocations. + Get-ChildItem -Path ([System.IO.Path]::GetTempPath()) -Filter 'pull-sdlc-self-*.ps1' -ErrorAction SilentlyContinue | + Remove-Item -Force -ErrorAction SilentlyContinue } } @@ -1587,7 +1647,7 @@ Describe 'Invoke-PullSDLC self-refresh wiring' { $fx = New-DiffReplayFixture -Root $script:fixtureRoot ` -Seed { 'a' | Out-File -Encoding utf8 CLAUDE.md -NoNewline } Mock -CommandName Test-SelfRefreshRequired -MockWith { return $true } - Mock -CommandName Invoke-SelfRefresh -MockWith { return $true } + Mock -CommandName Invoke-SelfRefresh -MockWith { return 'C:\fake\tmp.ps1' } Mock -CommandName Invoke-SelfReExec -MockWith { return 0 } # Even when self-refresh would say "go", Invoke-PullSDLC must not # invoke the gate -- it is the script's top-level responsibility now. @@ -1601,7 +1661,7 @@ Describe 'Invoke-PullSDLC self-refresh wiring' { Describe 'Invoke-SelfRefreshGate (issue #110)' { It 'short-circuits when Test-SelfRefreshRequired returns $false' { Mock -CommandName Test-SelfRefreshRequired -MockWith { return $false } - Mock -CommandName Invoke-SelfRefresh -MockWith { return $true } + Mock -CommandName Invoke-SelfRefresh -MockWith { return 'C:\fake\tmp.ps1' } Mock -CommandName Invoke-SelfReExec -MockWith { return 0 } $result = Invoke-SelfRefreshGate -ScriptPath 'C:\fake\Pull-SDLC.ai.ps1' -BoundParameters @{} $result | Should -BeFalse @@ -1609,9 +1669,9 @@ Describe 'Invoke-SelfRefreshGate (issue #110)' { Should -Invoke -CommandName Invoke-SelfReExec -Times 0 -Exactly } - It 'short-circuits when Invoke-SelfRefresh returns $false (hashes match / fetch failed)' { + It 'short-circuits when Invoke-SelfRefresh returns empty (hashes match / fetch failed)' { Mock -CommandName Test-SelfRefreshRequired -MockWith { return $true } - Mock -CommandName Invoke-SelfRefresh -MockWith { return $false } + Mock -CommandName Invoke-SelfRefresh -MockWith { return '' } Mock -CommandName Invoke-SelfReExec -MockWith { return 0 } $result = Invoke-SelfRefreshGate -ScriptPath 'C:\fake\Pull-SDLC.ai.ps1' -BoundParameters @{} $result | Should -BeFalse @@ -1620,7 +1680,7 @@ Describe 'Invoke-SelfRefreshGate (issue #110)' { It 're-execs via Invoke-SelfReExec when an update was applied' { Mock -CommandName Test-SelfRefreshRequired -MockWith { return $true } - Mock -CommandName Invoke-SelfRefresh -MockWith { return $true } + Mock -CommandName Invoke-SelfRefresh -MockWith { return 'C:\fake\tmp.ps1' } $script:reExecCount = 0 Mock -CommandName Invoke-SelfReExec -MockWith { $script:reExecCount++ @@ -1630,19 +1690,28 @@ Describe 'Invoke-SelfRefreshGate (issue #110)' { $script:reExecCount | Should -Be 1 } - It 'forwards exactly the supplied BoundParameters to Invoke-SelfReExec (regression for #110)' { + It 'passes the temp-file path (not the on-disk ScriptPath) to Invoke-SelfReExec (issue #180)' { Mock -CommandName Test-SelfRefreshRequired -MockWith { return $true } - Mock -CommandName Invoke-SelfRefresh -MockWith { return $true } - $script:capturedBound = $null + Mock -CommandName Invoke-SelfRefresh -MockWith { return 'C:\Temp\pull-sdlc-self-abc.ps1' } $script:capturedScriptPath = $null Mock -CommandName Invoke-SelfReExec -MockWith { param([string]$ScriptPath, [hashtable]$BoundParameters) $script:capturedScriptPath = $ScriptPath + } + $null = Invoke-SelfRefreshGate -ScriptPath 'C:\Consumer\Pull-SDLC.ai.ps1' -BoundParameters @{ Branch = 'main' } + $script:capturedScriptPath | Should -Be 'C:\Temp\pull-sdlc-self-abc.ps1' + } + + It 'forwards exactly the supplied BoundParameters to Invoke-SelfReExec (regression for #110)' { + Mock -CommandName Test-SelfRefreshRequired -MockWith { return $true } + Mock -CommandName Invoke-SelfRefresh -MockWith { return 'C:\fake\tmp.ps1' } + $script:capturedBound = $null + Mock -CommandName Invoke-SelfReExec -MockWith { + param([string]$ScriptPath, [hashtable]$BoundParameters) $script:capturedBound = $BoundParameters } $inputBound = @{ Branch = 'main'; RemoteName = 'sdlc.ai'; NoAutoPR = $true } $null = Invoke-SelfRefreshGate -ScriptPath 'C:\fake\Pull-SDLC.ai.ps1' -BoundParameters $inputBound - $script:capturedScriptPath | Should -Be 'C:\fake\Pull-SDLC.ai.ps1' # Captured keys must match input exactly -- no function-only leak (e.g. RemoteUrl). ($script:capturedBound.Keys | Sort-Object) -join ',' | Should -Be (($inputBound.Keys | Sort-Object) -join ',') $script:capturedBound.Keys | Should -Not -Contain 'RemoteUrl' diff --git a/Pull-SDLC.ai.ps1 b/Pull-SDLC.ai.ps1 index 1e99711..2b5c134 100644 --- a/Pull-SDLC.ai.ps1 +++ b/Pull-SDLC.ai.ps1 @@ -1209,7 +1209,46 @@ function Invoke-AutoWorktreeSync { # subsequent push will simply update it. # A worktree directory always contains a .git FILE (not directory). $worktreeMarker = Join-Path $absWorktree '.git' - $reusing = (Test-Path $worktreeMarker -PathType Leaf) + $markerExists = (Test-Path $worktreeMarker -PathType Leaf) + $reusing = $false + if ($markerExists) { + # Validate that the marker actually belongs to THIS repo. A + # leftover .worktrees/sdlc-sync/ directory can carry a .git file + # whose `gitdir:` line points to a different repo's (or a + # deleted) gitdir -- e.g. a copied tree, or a worktree directory + # that survived after the owning repo was relocated/removed. + # Trusting the marker in that case yields "fatal: not a git + # repository: (NULL)" for every subsequent git call inside the + # worktree (issue #180). Probe with `git rev-parse + # --git-common-dir` and compare against this repo's common-dir. + $thisCommonDir = (git rev-parse --git-common-dir 2>$null) + $thisCommonDirResolved = if ($thisCommonDir) { try { (Resolve-Path -LiteralPath $thisCommonDir -ErrorAction Stop).Path } catch { $null } } else { $null } + $reusing = $false + Push-Location $absWorktree + try { + $wtCommonDir = (git rev-parse --git-common-dir 2>$null) + if ($LASTEXITCODE -eq 0 -and $wtCommonDir) { + $wtCommonDirResolved = try { (Resolve-Path -LiteralPath $wtCommonDir -ErrorAction Stop).Path } catch { $null } + if ($wtCommonDirResolved -and $thisCommonDirResolved -and + ([string]::Equals($wtCommonDirResolved, $thisCommonDirResolved, [System.StringComparison]::OrdinalIgnoreCase))) { + $reusing = $true + } + } + } + finally { if ((Get-Location).Path -eq $absWorktree) { Pop-Location } } + + if (-not $reusing) { + Write-Host "Discarding stale worktree directory '$WorktreePath' (marker does not belong to this repo)." -ForegroundColor DarkGray + # Try a clean git-worktree removal first (works when this repo + # has a stale registration for the path). Fall back to a raw + # filesystem delete when the directory is orphaned. + git worktree remove --force $absWorktree 2>&1 | Out-Null + git worktree prune 2>&1 | Out-Null + if (Test-Path -LiteralPath $absWorktree) { + Remove-Item -LiteralPath $absWorktree -Recurse -Force -ErrorAction SilentlyContinue + } + } + } if ($reusing) { Push-Location $absWorktree try { @@ -1400,20 +1439,34 @@ function Invoke-SelfRefresh { <# .SYNOPSIS Fetches the upstream Pull-SDLC.ai.ps1 and, if its SHA256 differs from - the local copy, atomically replaces $ScriptPath with the new content. + the local copy, returns the path to a temp file containing the new + content. The caller re-execs from that temp file; the on-disk + $ScriptPath is intentionally NOT overwritten. .DESCRIPTION The fetch is cache-busted on every call (per-invocation query parameter plus Cache-Control / Pragma no-cache headers) to defeat Fastly stale hits on raw.githubusercontent.com -- otherwise a refresh issued within the CDN's TTL of a fresh upstream merge would silently no-op against - the previous body. Every outcome is logged on the Verbose stream so - '-Verbose' makes "did the refresh run, and what did it see?" - answerable from the output without grepping for a missing line. + the previous body. + + The on-disk script is left untouched (issue #180). Overwriting it on + `main` produced a dirty working tree on every upstream change and + silently destroyed any local edits. The upstream content is delivered + to the consumer through the worktree sync's PR; when the user merges + that PR and pulls, the on-disk script is updated through git like any + other managed file. + + Every outcome is logged on the Verbose stream so '-Verbose' makes + "did the refresh run, and what did it see?" answerable from the + output without grepping for a missing line. .OUTPUTS - [bool] $true if the local file was updated (caller should re-exec). - $false if hashes match, the fetch failed, or any error occurred. + [string] Absolute path to a temp file with the new upstream content, + when an update is available (caller should re-exec from that path). + Empty string when hashes match, the fetch failed, or any error + occurred. #> [CmdletBinding()] + [OutputType([string])] param( [Parameter(Mandatory)][string]$ScriptPath, [string]$Url = 'https://raw.githubusercontent.com/IntelliTect-Samples/IntelliSDLC.ai/main/Pull-SDLC.ai.ps1', @@ -1432,7 +1485,7 @@ function Invoke-SelfRefresh { Write-Warning "Self-update check skipped: $($_.Exception.Message)" Write-Verbose "Self-refresh: skipped (fetch failed: $($_.Exception.Message))" if (Test-Path -LiteralPath $tmp) { Remove-Item -LiteralPath $tmp -Force -ErrorAction SilentlyContinue -WhatIf:$false -Confirm:$false } - return $false + return '' } try { $remoteHash = (Get-FileHash -LiteralPath $tmp -Algorithm SHA256).Hash @@ -1440,26 +1493,30 @@ function Invoke-SelfRefresh { if ($remoteHash -eq $localHash) { Write-Verbose "Self-refresh: up-to-date (sha256=$($localHash.Substring(0,7)))" Remove-Item -LiteralPath $tmp -Force -ErrorAction SilentlyContinue -WhatIf:$false -Confirm:$false - return $false + return '' } Write-Host ("Self-updated Pull-SDLC.ai.ps1 from {0} to {1}; re-running with original args" -f $localHash.Substring(0, 7), $remoteHash.Substring(0, 7)) -ForegroundColor Cyan Write-Verbose "Self-refresh: updated $($localHash.Substring(0,7)) -> $($remoteHash.Substring(0,7))" - Move-Item -LiteralPath $tmp -Destination $ScriptPath -Force -WhatIf:$false -Confirm:$false - return $true + return $tmp } catch { Write-Warning "Self-update check skipped: $($_.Exception.Message)" if (Test-Path -LiteralPath $tmp) { Remove-Item -LiteralPath $tmp -Force -ErrorAction SilentlyContinue -WhatIf:$false -Confirm:$false } - return $false + return '' } } function Invoke-SelfReExec { <# .SYNOPSIS - Re-invokes the freshly self-updated Pull-SDLC.ai.ps1 with the caller's - original bound parameters, force-adding -NoSelfUpdate to prevent loops. - Exits the host process with the child script's exit code. + Re-invokes the freshly self-updated Pull-SDLC.ai.ps1 (typically a temp + file produced by Invoke-SelfRefresh) with the caller's original bound + parameters, force-adding -NoSelfUpdate to prevent loops. Exits the + host process with the child script's exit code. + .DESCRIPTION + $ScriptPath is the path to the script to run. With issue #180 this is + a temp file (the upstream-downloaded copy), not the consumer's on-disk + Pull-SDLC.ai.ps1. The temp file is removed after the child exits. #> [CmdletBinding()] param( @@ -1468,8 +1525,21 @@ function Invoke-SelfReExec { ) $reArgs = @{} + $BoundParameters $reArgs['NoSelfUpdate'] = $true - & $ScriptPath @reArgs - exit $LASTEXITCODE + try { + & $ScriptPath @reArgs + $childExit = $LASTEXITCODE + } + finally { + # Clean up the temp file produced by Invoke-SelfRefresh. Match the + # naming convention so we never delete an unrelated on-disk script. + $tempDir = [System.IO.Path]::GetTempPath() + $scriptDir = [System.IO.Path]::GetDirectoryName($ScriptPath) + $scriptName = [System.IO.Path]::GetFileName($ScriptPath) + if ($scriptDir -and ($scriptDir.TrimEnd([System.IO.Path]::DirectorySeparatorChar, [System.IO.Path]::AltDirectorySeparatorChar) -eq $tempDir.TrimEnd([System.IO.Path]::DirectorySeparatorChar, [System.IO.Path]::AltDirectorySeparatorChar)) -and $scriptName -like 'pull-sdlc-self-*.ps1') { + Remove-Item -LiteralPath $ScriptPath -Force -ErrorAction SilentlyContinue -WhatIf:$false -Confirm:$false + } + } + exit $childExit } function Write-NextStepsBanner { @@ -1546,8 +1616,9 @@ function Invoke-SelfRefreshGate { [switch]$NoSelfUpdate ) if (Test-SelfRefreshRequired -ScriptPath $ScriptPath -NoSelfUpdate:$NoSelfUpdate) { - if (Invoke-SelfRefresh -ScriptPath $ScriptPath) { - Invoke-SelfReExec -ScriptPath $ScriptPath -BoundParameters $BoundParameters + $newScriptPath = Invoke-SelfRefresh -ScriptPath $ScriptPath + if ($newScriptPath) { + Invoke-SelfReExec -ScriptPath $newScriptPath -BoundParameters $BoundParameters return $true } }