Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions scripts/lib/rebuild-test-deploy.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,24 @@ function Invoke-TestDeployRebuild {
}

$mainRefSpec = '+refs/heads/main:refs/remotes/origin/main'
Invoke-TestDeployGitCommand -Tool 'git' -Arguments @('fetch', 'origin', $mainRefSpec) -WorkingDirectory $deployRoot -CommandRunner $CommandRunner | Out-Null
Invoke-TestDeployGitCommand -Tool 'git' -Arguments @('reset', '--hard', 'origin/main') -WorkingDirectory $deployRoot -CommandRunner $CommandRunner | Out-Null
Invoke-TestDeployGitCommand -Tool 'git' -Arguments @('clean', '-fdx') -WorkingDirectory $deployRoot -CommandRunner $CommandRunner | Out-Null
$restoredEnvFiles = @(Restore-TestDeployEnvSnapshot -DeploymentPath $deployRoot -Snapshot $envSnapshot)
$restoredEnvFiles = @()
try {
Invoke-TestDeployGitCommand -Tool 'git' -Arguments @('fetch', 'origin', $mainRefSpec) -WorkingDirectory $deployRoot -CommandRunner $CommandRunner | Out-Null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep fetch outside the cleanup-recovery block

When the explicit git fetch fails because of network or auth, no reset or clean has run yet, but placing it inside this cleanup-recovery try means the catch still rewrites preserved .env snapshots. That changes the documented fail-fast fetch path and can even mask the real fetch blocker if a preserved env file is locked or unwritable; only failures after a destructive reset/clean step should trigger snapshot restoration.

Useful? React with 👍 / 👎.

Invoke-TestDeployGitCommand -Tool 'git' -Arguments @('reset', '--hard', 'origin/main') -WorkingDirectory $deployRoot -CommandRunner $CommandRunner | Out-Null
Invoke-TestDeployGitCommand -Tool 'git' -Arguments @('clean', '-fdx') -WorkingDirectory $deployRoot -CommandRunner $CommandRunner | Out-Null
$restoredEnvFiles = @(Restore-TestDeployEnvSnapshot -DeploymentPath $deployRoot -Snapshot $envSnapshot)
} catch {
$cleanupError = $_
try {
$restoredAfterFailure = @(Restore-TestDeployEnvSnapshot -DeploymentPath $deployRoot -Snapshot $envSnapshot)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Continue restoring later env snapshots after one write fails

In a partial cleanup failure where one preserved env file remains locked or unwritable while another has already been deleted, this single restore call aborts on the first WriteAllBytes error inside Restore-TestDeployEnvSnapshot. Because snapshots are restored in .env, coordinator .env, then host-kit order, a locked root .env prevents later deleted credentials such as .env.web-plane.host-kit from being restored, leaving the deployment checkout without the private MinIO settings this recovery path is meant to preserve.

Useful? React with 👍 / 👎.

if ($restoredAfterFailure.Count -gt 0) {
Write-Host "[rebuild-test-deploy] restored deployment env files after failed cleanup count=$($restoredAfterFailure.Count): $($restoredAfterFailure -join ', ')"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use structured logging instead of bare Write-Host in the new failure-restore path.

Line 272 introduces a new bare Write-Host; this should go through the structured logger used by scripts for consistent output and downstream parsing.

As per coding guidelines: "scripts/**/*.ps1: Use scripts/lib/StructLog.psm1 for structured logging output; do not replace with bare Write-Host calls".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/lib/rebuild-test-deploy.ps1` at line 272, Replace the bare Write-Host
call in the deployment restoration logging at line 272 with a structured logging
call from scripts/lib/StructLog.psm1. Instead of using Write-Host directly, use
the appropriate structured logging function from the StructLog module to log the
message about restored deployment environment files after failed cleanup. This
ensures consistent output formatting and enables downstream parsing as per the
scripts coding guidelines.

Source: Coding guidelines

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Emit cleanup recovery through structured logging

When git clean -fdx fails after deleting preserved env files, this new recovery message is emitted only via naked Write-Host, even though scripts/AGENTS.md explicitly requires scripts to use scripts/lib/StructLog.psm1 and not bare Write-Host. That makes this failure-recovery event invisible to the structured-log pipeline used for deploy diagnostics; please emit it through the structured logger instead.

Useful? React with 👍 / 👎.

}
} catch {
throw "$($cleanupError.Exception.Message)$([Environment]::NewLine)Additionally failed to restore preserved env files: $($_.Exception.Message)"
}
throw
}
if ($restoredEnvFiles.Count -gt 0) {
Write-Host "[rebuild-test-deploy] restored deployment env files count=$($restoredEnvFiles.Count): $($restoredEnvFiles -join ', ')"
}
Expand Down
53 changes: 53 additions & 0 deletions scripts/tests/test-rebuild-test-deploy.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,59 @@ try {
Assert-True ($script:cleanEvents.Count -eq 1) 'mock clean removed env files before restore'
Assert-Equal 3 $preserveResult.RestoredEnvFileCount 'rebuild restores current-version env files before deploy'

$cleanFailureRoot = Join-Path $sandbox 'clean-failure-root'
New-Item -ItemType Directory -Path (Join-Path $cleanFailureRoot '.git') -Force | Out-Null
New-Item -ItemType Directory -Path (Join-Path $cleanFailureRoot 'scripts') -Force | Out-Null
'deploy' | Set-Content -LiteralPath (Join-Path $cleanFailureRoot 'scripts\deploy.ps1') -Encoding ascii
'MINIO_WATCH_ENABLED=true' | Set-Content -LiteralPath (Join-Path $cleanFailureRoot '.env.web-plane.host-kit.example') -Encoding ascii
@(
'MINIO_WATCH_ENABLED=true',
'MINIO_WATCH_ACCESS_KEY=keep-after-failed-clean',
'MINIO_WATCH_SECRET_KEY=keep-secret-after-failed-clean'
) | Set-Content -LiteralPath (Join-Path $cleanFailureRoot '.env.web-plane.host-kit') -Encoding ascii

$cleanFailureCalls = New-Object 'System.Collections.Generic.List[string]'
$cleanFailureRunner = {
param([string] $Tool, [string[]] $Arguments, [string] $WorkingDirectory)
$script:cleanFailureCalls.Add("$Tool $($Arguments -join ' ') @ $WorkingDirectory")
$commandText = $Arguments -join ' '
if ($commandText -eq 'remote get-url origin') {
return [pscustomobject]@{ ExitCode = 0; Output = 'https://example.invalid/AI-BIM-governance.git' }
}
if ($commandText -eq 'rev-parse --short HEAD') {
return [pscustomobject]@{ ExitCode = 0; Output = 'abc1234' }
}
if ($commandText -eq 'status --short') {
return [pscustomobject]@{ ExitCode = 0; Output = '' }
}
if ($commandText -eq 'clean -fdx') {
Remove-Item -LiteralPath (Join-Path $WorkingDirectory '.env.web-plane.host-kit') -Force -ErrorAction Stop
return [pscustomobject]@{ ExitCode = 42; Output = 'locked governance log' }
}
return [pscustomobject]@{ ExitCode = 0; Output = 'ok' }
}.GetNewClosure()

$script:cleanFailureCalls = $cleanFailureCalls
$cleanFailureDeployWasCalled = $false
$cleanFailureDeployRunner = {
param([string] $DeployRoot)
$script:cleanFailureDeployWasCalled = $true
return [pscustomobject]@{ ExitCode = 0 }
}.GetNewClosure()

$cleanFailureMessage = $null
try {
Invoke-TestDeployRebuild -Build -RepoRoot $rebuildRoot -DeploymentPath $cleanFailureRoot -AllowNonFixedPathForTests -CommandRunner $cleanFailureRunner -DeployRunner $cleanFailureDeployRunner | Out-Null
} catch {
$cleanFailureMessage = $_.Exception.Message
}
Assert-True (-not [string]::IsNullOrWhiteSpace($cleanFailureMessage)) 'clean failure is surfaced'
Assert-True ($cleanFailureMessage -match 'locked governance log') 'clean failure includes command output'
Assert-True (-not $cleanFailureDeployWasCalled) 'clean failure stops before deploy'
$restoredHostKitEnv = Get-Content -LiteralPath (Join-Path $cleanFailureRoot '.env.web-plane.host-kit') -Raw
Assert-True ($restoredHostKitEnv -match 'MINIO_WATCH_ACCESS_KEY=keep-after-failed-clean') 'MinIO access key restored after failed clean'
Assert-True ($restoredHostKitEnv -match 'MINIO_WATCH_SECRET_KEY=keep-secret-after-failed-clean') 'MinIO secret key restored after failed clean'

$deployExitRoot = Join-Path $sandbox 'deploy-exit-root'
New-Item -ItemType Directory -Path (Join-Path $deployExitRoot '.git') -Force | Out-Null
New-Item -ItemType Directory -Path (Join-Path $deployExitRoot 'scripts') -Force | Out-Null
Expand Down
Loading