From d15816501b811a4abe410727021dee9908ca2024 Mon Sep 17 00:00:00 2001 From: "emre.kabaoglu" Date: Mon, 25 May 2026 13:21:55 +0300 Subject: [PATCH] fix(init): preserve other workflows on -Force re-init + unbreak --Force in workflow dispatcher (#442) --- scripts/init-project.ps1 | 49 +++++-- scripts/install-global.ps1 | 43 +++++- tests/Test-WorkflowIntegration.ps1 | 206 +++++++++++++++++++++++++++++ 3 files changed, 281 insertions(+), 17 deletions(-) diff --git a/scripts/init-project.ps1 b/scripts/init-project.ps1 index e05a79fb..282c62fe 100644 --- a/scripts/init-project.ps1 +++ b/scripts/init-project.ps1 @@ -217,20 +217,17 @@ function Invoke-BotFolderMigration { # had .bot/workflow.yaml plus old prompts/includes/research at .bot/recipes/. # None of these have a consumer in the new layout — # remove them so a re-init does not leave a confusing hybrid tree. + # + # Scope: ROOT ONLY. Do NOT recurse into .bot/workflows// — in the + # post-PR-5 layout those files (workflow.yaml, recipes/prompts/, etc.) are + # the canonical content of each installed workflow, not legacy residue. + # See issue #442. $legacyManifest = Join-Path $Dir "workflow.yaml" if (Test-Path $legacyManifest) { Remove-Item -Path $legacyManifest -Force } foreach ($legacy in @("recipes/prompts", "recipes/includes", "recipes/research")) { $legacyPath = Join-Path $Dir $legacy if (Test-Path $legacyPath) { Remove-Item -Path $legacyPath -Recurse -Force } } - - # Migrate installed workflow subdirectories - $wfDir = Join-Path $Dir "workflows" - if (Test-Path $wfDir) { - Get-ChildItem $wfDir -Directory | ForEach-Object { - Invoke-BotFolderMigration -Dir $_.FullName - } - } } # Run migration on existing .bot if present @@ -242,8 +239,11 @@ if (Test-Path $BotDir) { # Handle existing .bot with -Force (preserve workspace data) # --------------------------------------------------------------------------- $existingInstanceId = $null +$existingInstalledWorkflows = @() if ((Test-Path $BotDir) -and $Force) { - # Preserve instance_id before replacing settings/ + # Preserve instance_id and installed_workflows before replacing settings/. + # Without preserving installed_workflows, re-init with -Force forgets every + # workflow except the one being installed this run (issue #442). $existingSettingsPath = Join-Path $BotDir "settings\settings.default.json" if (Test-Path $existingSettingsPath) { try { @@ -254,6 +254,9 @@ if ((Test-Path $BotDir) -and $Force) { $existingInstanceId = $parsedGuid.ToString() } } + if ($existingSettings.PSObject.Properties['installed_workflows']) { + $existingInstalledWorkflows = @($existingSettings.installed_workflows) + } } catch { Write-DotbotCommand "Parse skipped: $_" } } @@ -544,12 +547,32 @@ if ($Workflow) { Write-Success "Installed workflow: $displayName" } } +} - # Record installed workflows in core settings - $settingsPath = Join-Path $BotDir "settings\settings.default.json" - if (Test-Path $settingsPath) { +# Record installed_workflows in core settings. Always runs so a re-init with +# -Force (with or without -Workflow) does not forget previously installed +# workflows preserved by $existingInstalledWorkflows (issue #442). Validate +# each entry still has a usable workflow dir on disk, otherwise stale entries +# (e.g. manually deleted workflows) would persist as ghosts. +$settingsPath = Join-Path $BotDir "settings\settings.default.json" +if (Test-Path $settingsPath) { + $combined = @($existingInstalledWorkflows) + @($installedWorkflows) + if ($combined.Count -gt 0) { $settings = Get-Content $settingsPath -Raw | ConvertFrom-Json - $settings | Add-Member -NotePropertyName "installed_workflows" -NotePropertyValue $installedWorkflows -Force + + $merged = @() + $seen = @{} + foreach ($wfName in $combined) { + if (-not $wfName) { continue } + $key = $wfName.ToLowerInvariant() + if ($seen.ContainsKey($key)) { continue } + $wfPath = Join-Path $BotDir "workflows\$wfName" + if (-not (Test-ValidWorkflowDir -Dir $wfPath)) { continue } + $seen[$key] = $true + $merged += $wfName + } + + $settings | Add-Member -NotePropertyName "installed_workflows" -NotePropertyValue $merged -Force $settings | ConvertTo-Json -Depth 10 | Set-Content $settingsPath } } diff --git a/scripts/install-global.ps1 b/scripts/install-global.ps1 index 30c131d2..0d7cae6e 100644 --- a/scripts/install-global.ps1 +++ b/scripts/install-global.ps1 @@ -292,18 +292,53 @@ function Invoke-Update { } function Invoke-Workflow { + # Parse: workflow add [--Force], workflow remove , workflow list $wfSubCmd = if ($SubArgs.Count -gt 0) { $SubArgs[0] } else { 'list' } - $wfName = if ($SubArgs.Count -gt 1) { $SubArgs[1] } else { '' } - [string[]]$wfExtra = @() - if ($SubArgs.Count -gt 2) { $wfExtra = @($SubArgs[2..($SubArgs.Count-1)]) } + # NB: [array] cast is required. Without it, PowerShell unwraps the @() + # returned from the if-expression when assigning a single-element array, + # so $wfRest ends up as a String and $wfRest[0] returns the first + # character. Same regression class as the "empty @wfExtra null" test + # in the CLI WORKFLOW ADD/REMOVE DISPATCH section. + [array]$wfRest = if ($SubArgs.Count -gt 1) { @($SubArgs[1..($SubArgs.Count-1)]) } else { @() } + $wfScript = switch ($wfSubCmd) { 'add' { Join-Path $ScriptsDir 'workflow-add.ps1' } 'remove' { Join-Path $ScriptsDir 'workflow-remove.ps1' } 'list' { Join-Path $ScriptsDir 'workflow-list.ps1' } default { $null } } + if ($wfScript -and (Test-Path $wfScript)) { - if ($wfExtra.Count -gt 0) { & $wfScript $wfName @wfExtra } else { & $wfScript $wfName } + # Separate positional args from named flags so switches like --Force / + # -Force bind as named parameters. Without this, array splat passes + # `--Force` literally and PowerShell errors with "A positional + # parameter cannot be found" (issue #442 workaround). + $wfSplat = @{} + $positional = @() + $i = 0 + while ($i -lt $wfRest.Count) { + if ($wfRest[$i] -match '^--?(.+)$') { + $pname = $Matches[1] + if (($i + 1) -lt $wfRest.Count -and $wfRest[$i + 1] -notmatch '^--?') { + $wfSplat[$pname] = $wfRest[$i + 1] + $i += 2 + } else { + $wfSplat[$pname] = $true + $i++ + } + } else { + $positional += $wfRest[$i] + $i++ + } + } + + # Map first positional to -Name for add/remove. workflow-list takes no + # positional args, so any stray positional there is silently ignored. + if ($wfSubCmd -in @('add', 'remove')) { + if ($positional.Count -ge 1) { $wfSplat['Name'] = $positional[0] } + } + + & $wfScript @wfSplat } else { Write-DotbotWarning "Usage: dotbot workflow [add|remove|list] [name] [--Force]" } diff --git a/tests/Test-WorkflowIntegration.ps1 b/tests/Test-WorkflowIntegration.ps1 index 65776c4d..cef25937 100644 --- a/tests/Test-WorkflowIntegration.ps1 +++ b/tests/Test-WorkflowIntegration.ps1 @@ -741,6 +741,212 @@ if ((Test-Path $wfAddScript) -and (Test-Path $startFromPromptDir)) { Write-TestResult -Name "workflow add functionality tests" -Status Skip -Message "workflow-add.ps1 or start-from-prompt workflow not found" } +# ═══════════════════════════════════════════════════════════════════ +# INIT -FORCE PRESERVES OTHER INSTALLED WORKFLOWS (issue #442) +# ═══════════════════════════════════════════════════════════════════ +# +# Regression: before the #442 fix, init -Force -Workflow X +# 1. wiped workflow.yaml + recipes/prompts in every other .bot/workflows//, +# and +# 2. overwrote installed_workflows with just [X]. +# Acceptance criteria from issue #442 are encoded as the asserts below. + +Write-Host "" +Write-Host " INIT -FORCE MULTI-WORKFLOW SAFETY (#442)" -ForegroundColor Cyan +Write-Host " ────────────────────────────────────────────" -ForegroundColor DarkGray + +$initScript = Join-Path $dotbotDir "scripts\init-project.ps1" +$wfAddScript442 = Join-Path $dotbotDir "scripts\workflow-add.ps1" +$startFromPromptSrc = Join-Path $dotbotDir "workflows\start-from-prompt" +$startFromJiraSrc = Join-Path $dotbotDir "workflows\start-from-jira" +$startFromRepoSrc = Join-Path $dotbotDir "workflows\start-from-repo" + +$have442Prereqs = (Test-Path $initScript) -and (Test-Path $wfAddScript442) -and + (Test-Path $startFromPromptSrc) -and (Test-Path $startFromJiraSrc) -and + (Test-Path $startFromRepoSrc) + +if ($have442Prereqs) { + # --- Test 1: init -Force -Workflow X preserves other workflows' files + settings list --- + $multiProj = New-TestProject + try { + Push-Location $multiProj + # Bootstrap: init with start-from-prompt + & pwsh -NoProfile -ExecutionPolicy Bypass -File $initScript -Workflow start-from-prompt 2>&1 | Out-Null + # Add a second workflow so we have a real multi-workflow state to defend + & pwsh -NoProfile -ExecutionPolicy Bypass -File $wfAddScript442 start-from-jira 2>&1 | Out-Null + Pop-Location + + $botDir = Join-Path $multiProj ".bot" + $jiraDir = Join-Path $botDir "workflows\start-from-jira" + $promptDir = Join-Path $botDir "workflows\start-from-prompt" + $settingsPath = Join-Path $botDir "settings\settings.default.json" + + # Sanity: pre-condition setup actually produced two workflows + Assert-PathExists -Name "#442 setup: start-from-jira installed before re-init" -Path (Join-Path $jiraDir "workflow.yaml") + $preSettings = Get-Content $settingsPath -Raw | ConvertFrom-Json + Assert-True -Name "#442 setup: both workflows in installed_workflows before re-init" ` + -Condition (('start-from-prompt' -in @($preSettings.installed_workflows)) -and ('start-from-jira' -in @($preSettings.installed_workflows))) ` + -Message "Pre-condition failed: expected both workflows installed; got $($preSettings.installed_workflows -join ',')" + + # Capture a recipe file count we can compare after re-init to detect + # the symptom-1 regression even if workflow.yaml were re-restored. + $jiraPromptsDir = Join-Path $jiraDir "recipes\prompts" + $jiraPromptsCountBefore = 0 + if (Test-Path $jiraPromptsDir) { + $jiraPromptsCountBefore = (Get-ChildItem -Path $jiraPromptsDir -File -ErrorAction SilentlyContinue).Count + } + + # Action under test + Push-Location $multiProj + & pwsh -NoProfile -ExecutionPolicy Bypass -File $initScript -Force -Workflow start-from-repo 2>&1 | Out-Null + Pop-Location + + # Symptom 1: file deletion + Assert-PathExists -Name "#442: start-from-jira/workflow.yaml survives init -Force -Workflow X" ` + -Path (Join-Path $jiraDir "workflow.yaml") + Assert-PathExists -Name "#442: start-from-jira/recipes/prompts/ survives init -Force -Workflow X" ` + -Path $jiraPromptsDir + $jiraPromptsCountAfter = 0 + if (Test-Path $jiraPromptsDir) { + $jiraPromptsCountAfter = (Get-ChildItem -Path $jiraPromptsDir -File -ErrorAction SilentlyContinue).Count + } + Assert-True -Name "#442: start-from-jira/recipes/prompts/ still has the same file count" ` + -Condition ($jiraPromptsCountAfter -eq $jiraPromptsCountBefore -and $jiraPromptsCountAfter -gt 0) ` + -Message "Prompt count changed: before=$jiraPromptsCountBefore after=$jiraPromptsCountAfter" + Assert-PathExists -Name "#442: start-from-prompt/workflow.yaml survives init -Force" ` + -Path (Join-Path $promptDir "workflow.yaml") + + # Symptom 2: installed_workflows overwrite + $postSettings = Get-Content $settingsPath -Raw | ConvertFrom-Json + $postList = @($postSettings.installed_workflows) + Assert-True -Name "#442: installed_workflows contains pre-existing 'start-from-jira' after re-init" ` + -Condition ('start-from-jira' -in $postList) ` + -Message "Lost: $($postList -join ',')" + Assert-True -Name "#442: installed_workflows contains pre-existing 'start-from-prompt' after re-init" ` + -Condition ('start-from-prompt' -in $postList) ` + -Message "Lost: $($postList -join ',')" + Assert-True -Name "#442: installed_workflows contains newly-installed 'start-from-repo'" ` + -Condition ('start-from-repo' -in $postList) ` + -Message "Got: $($postList -join ',')" + + # No duplicates introduced + $uniqueCount = ($postList | Select-Object -Unique).Count + Assert-True -Name "#442: installed_workflows has no duplicates after re-init" ` + -Condition ($uniqueCount -eq $postList.Count) ` + -Message "Duplicates: $($postList -join ',')" + + # --- Test 2: second re-init is a no-op (idempotent) --- + Push-Location $multiProj + & pwsh -NoProfile -ExecutionPolicy Bypass -File $initScript -Force -Workflow start-from-repo 2>&1 | Out-Null + Pop-Location + + $idemSettings = Get-Content $settingsPath -Raw | ConvertFrom-Json + $idemList = @($idemSettings.installed_workflows) + Assert-True -Name "#442: repeat init -Force is idempotent for installed_workflows" ` + -Condition (($idemList.Count -eq $postList.Count) -and (($idemList | Sort-Object) -join ',' -eq ($postList | Sort-Object) -join ',')) ` + -Message "Changed: before=$($postList -join ',') after=$($idemList -join ',')" + Assert-PathExists -Name "#442: repeat init -Force keeps start-from-jira/workflow.yaml" ` + -Path (Join-Path $jiraDir "workflow.yaml") + } finally { + Remove-TestProject -Path $multiProj + } + + # --- Test 3: fresh-project init -Force still works --- + $freshProj = New-TestProject + try { + Push-Location $freshProj + & pwsh -NoProfile -ExecutionPolicy Bypass -File $initScript -Force -Workflow start-from-prompt 2>&1 | Out-Null + Pop-Location + + $freshBot = Join-Path $freshProj ".bot" + $freshSettings = Get-Content (Join-Path $freshBot "settings\settings.default.json") -Raw | ConvertFrom-Json + $freshList = @($freshSettings.installed_workflows) + Assert-True -Name "#442: fresh init -Force -Workflow X gives installed_workflows=[X]" ` + -Condition ($freshList.Count -eq 1 -and $freshList[0] -eq 'start-from-prompt') ` + -Message "Got: $($freshList -join ',')" + } finally { + Remove-TestProject -Path $freshProj + } + + # --- Test 4: legacy root-level migration still runs --- + # Pre-PR-5 residue at .bot/workflow.yaml and .bot/recipes/* must still be + # cleaned by init -Force (the recursive call into workflows// was + # the bug; the root-level cleanup is intentional and must survive). + $legacyProj = New-TestProject + try { + Push-Location $legacyProj + & pwsh -NoProfile -ExecutionPolicy Bypass -File $initScript -Workflow start-from-prompt 2>&1 | Out-Null + Pop-Location + + $legacyBot = Join-Path $legacyProj ".bot" + # Plant pre-PR-5 residue at .bot/ root + Set-Content -Path (Join-Path $legacyBot "workflow.yaml") -Value "name: legacy-root" -Force + $rootRecipes = Join-Path $legacyBot "recipes\prompts" + New-Item -ItemType Directory -Path $rootRecipes -Force | Out-Null + Set-Content -Path (Join-Path $rootRecipes "stale.md") -Value "stale" -Force + + Push-Location $legacyProj + & pwsh -NoProfile -ExecutionPolicy Bypass -File $initScript -Force -Workflow start-from-prompt 2>&1 | Out-Null + Pop-Location + + Assert-PathNotExists -Name "#442: root-level legacy .bot/workflow.yaml is still removed" ` + -Path (Join-Path $legacyBot "workflow.yaml") + Assert-PathNotExists -Name "#442: root-level legacy .bot/recipes/prompts/ is still removed" ` + -Path $rootRecipes + } finally { + Remove-TestProject -Path $legacyProj + } +} else { + Write-TestResult -Name "#442 init -Force multi-workflow safety tests" -Status Skip ` + -Message "Required scripts/workflows not present: init-project / workflow-add / start-from-{prompt,jira,repo}" +} + +# ═══════════════════════════════════════════════════════════════════ +# CLI DISPATCHER NORMALIZES --Force (issue #442 workaround) +# ═══════════════════════════════════════════════════════════════════ +# +# Regression: `dotbot workflow add --Force` errored with "A positional +# parameter cannot be found that accepts argument '--Force'" because the +# dispatcher passed switches via array splat instead of hashtable splat. + +Write-Host "" +Write-Host " CLI DISPATCHER --Force NORMALIZATION (#442)" -ForegroundColor Cyan +Write-Host " ────────────────────────────────────────────" -ForegroundColor DarkGray + +$cliScript442 = Join-Path $dotbotDir "bin\dotbot.ps1" +if ((Test-Path $cliScript442) -and (Test-Path $startFromPromptSrc)) { + $dispProj = New-TestProjectFromGolden -Flavor 'default' + $dispProjPath = $dispProj.ProjectRoot + try { + # Pre-install once so the second call needs --Force to overwrite + & pwsh -NoProfile -ExecutionPolicy Bypass -Command "Set-Location '$dispProjPath'; & '$cliScript442' workflow add start-from-prompt" 2>&1 | Out-Null + + # The failing scenario before the fix: --Force (double-dash) blows up + $forceOutput = & pwsh -NoProfile -ExecutionPolicy Bypass -Command "Set-Location '$dispProjPath'; & '$cliScript442' workflow add start-from-prompt --Force" 2>&1 + $forceFailed = $forceOutput | Where-Object { + $_ -match 'positional parameter cannot be found' -or + $_ -match "cannot be found that accepts argument '--Force'" + } + Assert-True -Name "#442: 'dotbot workflow add X --Force' does not error" ` + -Condition ($null -eq $forceFailed -or $forceFailed.Count -eq 0) ` + -Message "Dispatcher dropped switch: $forceFailed" + + # And the single-dash form still works + $forceOutput2 = & pwsh -NoProfile -ExecutionPolicy Bypass -Command "Set-Location '$dispProjPath'; & '$cliScript442' workflow add start-from-prompt -Force" 2>&1 + $forceFailed2 = $forceOutput2 | Where-Object { + $_ -match 'positional parameter cannot be found' -or + $_ -match "cannot be found that accepts argument '-Force'" + } + Assert-True -Name "#442: 'dotbot workflow add X -Force' does not error" ` + -Condition ($null -eq $forceFailed2 -or $forceFailed2.Count -eq 0) ` + -Message "Dispatcher dropped switch: $forceFailed2" + } finally { + Remove-TestProject -Path $dispProjPath + } +} else { + Write-TestResult -Name "#442 CLI dispatcher --Force tests" -Status Skip -Message "dotbot CLI or start-from-prompt workflow not found" +} + # ═══════════════════════════════════════════════════════════════════ # DEFAULT WORKFLOW RESOLUTION # ═══════════════════════════════════════════════════════════════════