Conversation
…factor default model fallback
# Conflicts: # profiles/default/systems/runtime/launch-process.ps1
There was a problem hiding this comment.
Pull request overview
This PR refactors the runtime launch flow into a modular “process registry + process type handlers” architecture, and adds a dedicated analyse process for scanning an existing repo and generating foundational product documents.
Changes:
- Introduces reusable runtime modules for process persistence/logging/locking (
ProcessRegistry.psm1) and shared task-loop helpers (TaskLoop.psm1). - Splits the previously monolithic
launch-process.ps1logic into process-type implementations (analysis/execution/workflow/kickstart/prompt/analyse) and dispatches by-Type. - Adds an
Invoke-AnalyseProcessprocess type that prompts the model to scan an existing repository and writemission.md,tech-stack.md, andentity-model.md.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| profiles/default/systems/runtime/modules/TaskLoop.psm1 | New shared helpers for YAML front matter, task selection, deadlock detection, and the interview loop. |
| profiles/default/systems/runtime/modules/ProcessTypes/Invoke-WorkflowProcess.ps1 | New unified “analyse then execute” workflow process implementation. |
| profiles/default/systems/runtime/modules/ProcessTypes/Invoke-PromptProcess.ps1 | New prompt-only process handler for planning/commit/task-creation flows. |
| profiles/default/systems/runtime/modules/ProcessTypes/Invoke-KickstartProcess.ps1 | New kickstart pipeline handler with phase tracking and optional workflow spawning. |
| profiles/default/systems/runtime/modules/ProcessTypes/Invoke-ExecutionProcess.ps1 | New execution loop handler (worktrees, retries, completion checks, merge). |
| profiles/default/systems/runtime/modules/ProcessTypes/Invoke-AnalysisProcess.ps1 | New analysis loop handler for task analysis (rate limit handling, status transitions). |
| profiles/default/systems/runtime/modules/ProcessTypes/Invoke-AnalyseProcess.ps1 | New “analyse existing repo” handler that generates core product documents. |
| profiles/default/systems/runtime/modules/ProcessRegistry.psm1 | New module for process ID creation, process file writing, activity logging, locking, diagnostics, and preflight checks. |
| profiles/default/systems/runtime/launch-process.ps1 | Updated to import the new modules/handlers, remove inlined helpers, and dispatch by process type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ($mergeResult.push_result.success) { | ||
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Pushed to remote: $($task.name)" -ProcessesDir $ProcessesDir | ||
| } else { | ||
| Write-Status "Push failed: $($mergeResult.push_result.error)" -Type Warning | ||
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Push failed after merge: $($mergeResult.push_result.error)" -ProcessesDir $ProcessesDir | ||
| } | ||
| } |
There was a problem hiding this comment.
Write-Status's -Type ValidateSet does not include Warning, so this call will throw and can interrupt workflow completion (especially after a successful merge when only the push failed). Use -Type Warn (or expand Write-Status to support Warning).
| if ($mergeResult.push_result.success) { | |
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Pushed to remote: $($task.name)" -ProcessesDir $ProcessesDir | |
| } else { | |
| Write-Status "Push failed: $($mergeResult.push_result.error)" -Type Warning | |
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Push failed after merge: $($mergeResult.push_result.error)" -ProcessesDir $ProcessesDir | |
| } | |
| } | |
| if ($mergeResult.push_result.success) { | |
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Pushed to remote: $($task.name)" -ProcessesDir $ProcessesDir | |
| } else { | |
| Write-Status "Push failed: $($mergeResult.push_result.error)" -Type Warn | |
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Push failed after merge: $($mergeResult.push_result.error)" -ProcessesDir $ProcessesDir | |
| } | |
| } |
| function Test-ProcessLock { | ||
| [CmdletBinding()] | ||
| [OutputType([bool])] | ||
| param( | ||
| [Parameter(Mandatory)][string]$LockType, | ||
| [Parameter(Mandatory)][string]$ControlDir | ||
| ) | ||
| $lockPath = Join-Path $ControlDir "launch-$LockType.lock" | ||
| if (-not (Test-Path $lockPath)) { return $false } | ||
| $lockContent = Get-Content $lockPath -Raw -ErrorAction SilentlyContinue | ||
| if (-not $lockContent) { return $false } | ||
| try { | ||
| Get-Process -Id ([int]$lockContent.Trim()) -ErrorAction Stop | Out-Null | ||
| return $true | ||
| } catch { | ||
| Remove-Item $lockPath -Force -ErrorAction SilentlyContinue | ||
| return $false | ||
| } | ||
| } | ||
|
|
||
| function Set-ProcessLock { | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(Mandatory)][string]$LockType, | ||
| [Parameter(Mandatory)][string]$ControlDir | ||
| ) | ||
| $lockPath = Join-Path $ControlDir "launch-$LockType.lock" | ||
| $PID.ToString() | Set-Content $lockPath -NoNewline -Encoding utf8NoBOM | ||
| } |
There was a problem hiding this comment.
The lock implementation is not atomic: two processes can both pass Test-ProcessLock and then both call Set-ProcessLock, allowing concurrent runs and/or having one process remove another's lock in cleanup. Consider creating the lock file atomically (e.g., open with FileMode.CreateNew / New-Item -ErrorAction Stop) and making Remove-ProcessLock verify the PID matches the current process before deleting.
| function Write-ProcessFile { | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(Mandatory)][string]$Id, | ||
| [Parameter(Mandatory)][hashtable]$Data, | ||
| [Parameter(Mandatory)][string]$ProcessesDir | ||
| ) | ||
| $filePath = Join-Path $ProcessesDir "$Id.json" | ||
| $tempFile = "$filePath.tmp" | ||
|
|
||
| $maxRetries = 3 | ||
| for ($r = 0; $r -lt $maxRetries; $r++) { | ||
| try { | ||
| $Data | ConvertTo-Json -Depth 10 | Set-Content -Path $tempFile -Encoding utf8NoBOM -NoNewline | ||
| Move-Item -Path $tempFile -Destination $filePath -Force -ErrorAction Stop | ||
| return | ||
| } catch { | ||
| if (Test-Path $tempFile) { Remove-Item $tempFile -Force -ErrorAction SilentlyContinue } | ||
| if ($r -lt ($maxRetries - 1)) { | ||
| Start-Sleep -Milliseconds (50 * ($r + 1)) | ||
| } else { | ||
| Write-Diag -Msg "Write-ProcessFile FAILED for $Id after $maxRetries retries: $_" -DiagLogPath $script:DiagLogPath | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Write-ProcessFile silently gives up after exhausting retries (it only logs to diag). Callers assume the process state was persisted, so a write failure can lead to a running process with no/partial registry state and hard-to-debug behavior. Consider throwing a terminating error after the final retry (or returning a success flag that callers must check).
| $existing = Get-Content $FilePath -Raw | ||
| ($yaml + $existing) | Set-Content -Path $FilePath -Encoding utf8NoBOM -NoNewline |
There was a problem hiding this comment.
Add-YamlFrontMatter always prepends a new front matter block without checking whether one already exists. Re-running kickstart phases (or re-generating docs) can produce duplicated --- blocks, which many front-matter parsers treat as invalid/ambiguous. Consider detecting an existing front-matter header at the start of the file and replacing/merging instead of blindly prepending.
| $existing = Get-Content $FilePath -Raw | |
| ($yaml + $existing) | Set-Content -Path $FilePath -Encoding utf8NoBOM -NoNewline | |
| $existing = Get-Content -Path $FilePath -Raw | |
| # If the file already starts with a YAML front matter block, replace it; | |
| # otherwise, prepend the new front matter. | |
| $newContent = $null | |
| if ($existing) { | |
| # Normalize line splitting across different newline conventions | |
| $lines = $existing -split "(`r`n|`n|`r)" | |
| if ($lines.Length -gt 0 -and $lines[0].Trim() -eq '---') { | |
| # Find the closing '---' that terminates the existing front matter | |
| $closingIndex = $null | |
| for ($i = 1; $i -lt $lines.Length; $i++) { | |
| if ($lines[$i].Trim() -eq '---') { | |
| $closingIndex = $i | |
| break | |
| } | |
| } | |
| if ($null -ne $closingIndex) { | |
| # Preserve everything after the existing front matter block | |
| if ($closingIndex -lt ($lines.Length - 1)) { | |
| $bodyLines = $lines[($closingIndex + 1)..($lines.Length - 1)] | |
| $body = ($bodyLines -join "`n") | |
| } else { | |
| $body = "" | |
| } | |
| $newContent = $yaml + $body | |
| } | |
| } | |
| } | |
| if ($null -eq $newContent) { | |
| # No existing, well-formed leading front matter detected; prepend ours. | |
| $newContent = $yaml + $existing | |
| } | |
| $newContent | Set-Content -Path $FilePath -Encoding utf8NoBOM -NoNewline |
| try { | ||
| $streamArgs = @{ | ||
| Prompt = $fullPrompt | ||
| Model = $ClaudeModelName | ||
| SessionId = $ClaudeSessionId | ||
| PersistSession = $false | ||
| } | ||
| if ($ShowDebug) { $streamArgs['ShowDebugJson'] = $true } | ||
| if ($ShowVerbose) { $streamArgs['ShowVerbose'] = $true } | ||
|
|
||
| Invoke-ProviderStream @streamArgs | ||
|
|
||
| $processData.status = 'completed' | ||
| $processData.completed_at = (Get-Date).ToUniversalTime().ToString("o") | ||
| $processData.heartbeat_status = "Completed: $Description" | ||
| } catch { | ||
| $processData.status = 'failed' | ||
| $processData.failed_at = (Get-Date).ToUniversalTime().ToString("o") | ||
| $processData.error = $_.Exception.Message | ||
| $processData.heartbeat_status = "Failed: $($_.Exception.Message)" | ||
| Write-Status "Process failed: $($_.Exception.Message)" -Type Error | ||
| } | ||
|
|
||
| Write-ProcessFile -Id $ProcId -Data $processData -ProcessesDir $ProcessesDir | ||
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Process $ProcId finished ($($processData.status))" -ProcessesDir $ProcessesDir | ||
| } |
There was a problem hiding this comment.
This process uses a provider session ID but never calls Remove-ProviderSession. Other process types clean up session artifacts (e.g., task analysis/execution), so repeated runs of prompt-based processes can accumulate stale session data under the provider's local storage (Claude: ~/.claude/projects/...). Consider adding a ProjectRoot parameter (or deriving it from BotRoot) and cleaning up the session in a finally block.
| $streamArgs = @{ | ||
| Prompt = $analysePrompt | ||
| Model = $ClaudeModelName | ||
| SessionId = $ClaudeSessionId | ||
| PersistSession = $false | ||
| } | ||
| if ($ShowDebug) { $streamArgs['ShowDebugJson'] = $true } | ||
| if ($ShowVerbose) { $streamArgs['ShowVerbose'] = $true } | ||
|
|
||
| Invoke-ProviderStream @streamArgs | ||
|
|
||
| # Verify product docs were created | ||
| $hasDocs = (Test-Path (Join-Path $productDir "mission.md")) -and | ||
| (Test-Path (Join-Path $productDir "tech-stack.md")) -and | ||
| (Test-Path (Join-Path $productDir "entity-model.md")) | ||
|
|
||
| if (-not $hasDocs) { | ||
| throw "Analyse failed: product documents were not created" | ||
| } | ||
|
|
||
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Analyse complete - product documents created" -ProcessesDir $ProcessesDir | ||
|
|
||
| $processData.status = 'completed' | ||
| $processData.completed_at = (Get-Date).ToUniversalTime().ToString("o") | ||
| $processData.heartbeat_status = "Completed: $Description" | ||
| } catch { | ||
| $processData.status = 'failed' | ||
| $processData.failed_at = (Get-Date).ToUniversalTime().ToString("o") | ||
| $processData.error = $_.Exception.Message | ||
| $processData.heartbeat_status = "Failed: $($_.Exception.Message)" | ||
| Write-Status "Process failed: $($_.Exception.Message)" -Type Error | ||
| } | ||
|
|
||
| Write-ProcessFile -Id $ProcId -Data $processData -ProcessesDir $ProcessesDir | ||
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Process $ProcId finished ($($processData.status))" -ProcessesDir $ProcessesDir | ||
| } |
There was a problem hiding this comment.
This process runs the provider with a session ID but never calls Remove-ProviderSession, so Claude session artifacts can accumulate over time. Consider cleaning up the session in a finally block after completion/failure (you can derive ProjectRoot from BotRoot if you don't want to thread it as a parameter).
| if ($mergeResult.push_result.success) { | ||
| Write-ProcessActivity -Id $ProcId -ActivityType "text" -Message "Pushed to remote: $($task.name)" -ProcessesDir $ProcessesDir | ||
| } else { | ||
| Write-Status "Push failed: $($mergeResult.push_result.error)" -Type Warning |
There was a problem hiding this comment.
Write-Status only supports Type values {Info, Success, Error, Warn, Process, Complete} (see DotBotTheme.psm1). Using -Type Warning will trigger a parameter validation error at runtime, potentially breaking the post-merge push failure path. Use -Type Warn (or add Warning to Write-Status's ValidateSet if intentional).
| Write-Status "Push failed: $($mergeResult.push_result.error)" -Type Warning | |
| Write-Status "Push failed: $($mergeResult.push_result.error)" -Type Warn |
Resolves: https://github.com/andresharpe/dotbot/blob/claude/dotbot-refactor-planning-kKO9U/docs/DOTBOT-V4-phase-03-launch-process-breakup.md
This pull request introduces a new modular process registry and a dedicated analysis process type for the runtime system. The main changes are the addition of a reusable PowerShell module for process registry operations (including locking, logging, and diagnostics), and a new
Invoke-AnalyseProcessimplementation that formalizes and automates the product analysis workflow for existing projects.Process Registry Module Enhancements:
ProcessRegistry.psm1module providing reusable functions for process management, including process file CRUD, activity logging, lock handling, diagnostics, and preflight checks. These functions support robust process tracking and coordination in the runtime environment.New-ProcessId,Write-ProcessFile,Write-ProcessActivity,Test-ProcessLock,Set-ProcessLock,Remove-ProcessLock, andTest-Preflightfor use across the system.Analysis Process Implementation:
Invoke-AnalyseProcess.ps1, which defines a new process type for analyzing an existing codebase and generating foundational product documents (mission.md,tech-stack.md,entity-model.md). The function orchestrates the workflow, manages process state, and logs activity.These changes lay the groundwork for robust, modular process management and standardized, automated analysis workflows in the runtime system.