Make build/test worktree-aware with same-worktree lock

Author: johnml1135

.github/instructions/build.instructions.md

@@ -44,6 +44,21 @@ Key ordering constraint:
 ## Worktrees and concurrent builds
 This repo supports multiple concurrent builds across git worktrees. Prefer the scripts because they handle environment setup and avoid cross-worktree process conflicts.
 
+### Worktree-aware process cleanup
+- `build.ps1` now scopes process cleanup to the current repository root (`$PSScriptRoot`).
+- This avoids killing active `msbuild`/`dotnet`/test processes started from other worktrees.
+- The same worktree is intentionally exclusive: `build.ps1`/`test.ps1` acquire a named lock and fail fast if another build/test workflow is already running in that worktree.
+- Lock metadata is written to `Output/WorktreeRun.lock.json` while the workflow is active.
+- Optional actor tagging: set `FW_BUILD_STARTED_BY=user|agent` (or pass `-StartedBy`) to record who owns the lock.
+
+### MSBuild node reuse default
+- `build.ps1` defaults `-NodeReuse` to `false` to reduce cross-worktree lock contention from shared MSBuild worker nodes.
+- If you prefer faster local inner-loop builds and are not running concurrent worktrees, you can opt back in with:
+
+```powershell
+.\build.ps1 -NodeReuse $true
+```
+
 ## Troubleshooting (common)
 
 ### “Native artifacts missing” / code-generation failures

.github/instructions/testing.instructions.md

@@ -65,6 +65,13 @@ The testing infrastructure relies on shared PowerShell modules for consistency:
 -   **`Build/scripts/Invoke-CppTest.ps1`**: Backend for native C++ tests (MSBuild/NMake).
 -   **`Build/Agent/FwBuildHelpers.psm1`**: Shared logic for VS environment, and process cleanup.
 
+## Worktree-aware behavior
+- `test.ps1` scopes process cleanup to the current repository root (`$PSScriptRoot`) so concurrent runs in other worktrees are not terminated.
+- `build.ps1` and `test.ps1` in the same worktree both target `Output/<Configuration>/`; a same-worktree lock prevents concurrent runs and fails fast with owner details.
+- `build.ps1 -RunTests` is supported in the same worktree: the child `test.ps1` run reuses the parent workflow lock.
+- If you need concurrency, use separate git worktrees and run one scripted workflow per worktree.
+- Optional lock ownership tagging: set `FW_BUILD_STARTED_BY=user|agent` (or pass `-StartedBy`) before running scripts.
+
 ## Debugging & Logs
 -   **Logs**: Build logs are written to `Output/Build.log` (if configured) or standard output.
 -   **Verbosity**: Use `-Verbosity detailed` with `test.ps1` for more output.

Build/Agent/FwBuildHelpers.psm1

@@ -125,6 +125,159 @@ function Stop-ConflictingProcesses {
     }
 }
 
+function Get-WorktreeMutexName {
+    param([Parameter(Mandatory)][string]$RepoRoot)
+
+    $normalizedRepoRoot = [System.IO.Path]::GetFullPath($RepoRoot).TrimEnd('\\', '/')
+    $normalizedRepoRoot = $normalizedRepoRoot.ToLowerInvariant()
+
+    $bytes = [System.Text.Encoding]::UTF8.GetBytes($normalizedRepoRoot)
+    $sha = [System.Security.Cryptography.SHA256]::Create()
+    try {
+        $hashBytes = $sha.ComputeHash($bytes)
+    }
+    finally {
+        $sha.Dispose()
+    }
+
+    $hash = [System.BitConverter]::ToString($hashBytes).Replace('-', '')
+    $shortHash = $hash.Substring(0, 16)
+    return "Global\\FieldWorks.Worktree.$shortHash"
+}
+
+function Enter-WorktreeLock {
+    <#
+    .SYNOPSIS
+        Acquires an exclusive same-worktree lock for build/test workflows.
+    .DESCRIPTION
+        Uses a named Windows mutex keyed by RepoRoot to prevent concurrent build/test
+        runs inside the same worktree. Also writes lock metadata to Output/ so users
+        can see who currently owns the lock.
+    .PARAMETER RepoRoot
+        Repository root path for this worktree.
+    .PARAMETER Context
+        Friendly text describing the operation (e.g. "FieldWorks build").
+    .PARAMETER StartedBy
+        Optional actor tag for diagnostics (for example: user, agent).
+    #>
+    param(
+        [Parameter(Mandatory)][string]$RepoRoot,
+        [Parameter(Mandatory)][string]$Context,
+        [string]$StartedBy = 'unknown'
+    )
+
+    $mutexName = Get-WorktreeMutexName -RepoRoot $RepoRoot
+    $mutex = New-Object System.Threading.Mutex($false, $mutexName)
+    $hasHandle = $false
+
+    try {
+        try {
+            $hasHandle = $mutex.WaitOne(0)
+        }
+        catch [System.Threading.AbandonedMutexException] {
+            $hasHandle = $true
+        }
+
+        $lockPath = Join-Path $RepoRoot "Output\\WorktreeRun.lock.json"
+
+        if (-not $hasHandle) {
+            $ownerDetails = $null
+            if (Test-Path -LiteralPath $lockPath -PathType Leaf) {
+                try {
+                    $ownerDetails = Get-Content -LiteralPath $lockPath -Raw | ConvertFrom-Json
+                }
+                catch {
+                    $ownerDetails = $null
+                }
+            }
+
+            if ($ownerDetails) {
+                throw "$Context is already running in this worktree (ownerPid=$($ownerDetails.Pid), startedBy=$($ownerDetails.StartedBy), startedAtUtc=$($ownerDetails.StartedAtUtc), context=$($ownerDetails.Context)). Run one build/test workflow at a time per worktree."
+            }
+
+            throw "$Context is already running in this worktree. Run one build/test workflow at a time per worktree."
+        }
+
+        $outputDir = Split-Path -Parent $lockPath
+        if (-not (Test-Path -LiteralPath $outputDir -PathType Container)) {
+            New-Item -Path $outputDir -ItemType Directory -Force | Out-Null
+        }
+
+        $metadata = [pscustomobject]@{
+            Context = $Context
+            StartedBy = $StartedBy
+            Pid = $PID
+            ProcessName = (Get-Process -Id $PID).ProcessName
+            RepoRoot = [System.IO.Path]::GetFullPath($RepoRoot)
+            MutexName = $mutexName
+            StartedAtUtc = (Get-Date).ToUniversalTime().ToString('o')
+        }
+
+        $metadata | ConvertTo-Json -Depth 4 | Set-Content -LiteralPath $lockPath -Encoding UTF8
+
+        return [pscustomobject]@{
+            Mutex = $mutex
+            MutexName = $mutexName
+            LockPath = $lockPath
+            HasHandle = $true
+            OwnerPid = $PID
+        }
+    }
+    catch {
+        if (-not $hasHandle -and $mutex) {
+            $mutex.Dispose()
+        }
+        throw
+    }
+}
+
+function Exit-WorktreeLock {
+    <#
+    .SYNOPSIS
+        Releases a same-worktree lock acquired by Enter-WorktreeLock.
+    #>
+    param([Parameter(Mandatory)]$LockHandle)
+
+    if (-not $LockHandle) {
+        return
+    }
+
+    $mutex = $LockHandle.Mutex
+    $lockPath = $LockHandle.LockPath
+
+    try {
+        if ($lockPath -and (Test-Path -LiteralPath $lockPath -PathType Leaf)) {
+            try {
+                $owner = $null
+                try {
+                    $owner = Get-Content -LiteralPath $lockPath -Raw | ConvertFrom-Json
+                }
+                catch {
+                    $owner = $null
+                }
+
+                if (-not $owner -or ($owner.Pid -eq $PID)) {
+                    Remove-Item -LiteralPath $lockPath -Force -ErrorAction SilentlyContinue
+                }
+            }
+            catch {
+                # Best effort only
+            }
+        }
+    }
+    finally {
+        if ($mutex) {
+            try {
+                $mutex.ReleaseMutex()
+            }
+            catch {
+                # Ignore release failures
+            }
+            $mutex.Dispose()
+        }
+    }
+}
+
 function Test-IsFileLockError {
     param([Parameter(Mandatory)][System.Management.Automation.ErrorRecord]$ErrorRecord)
 
@@ -156,10 +309,28 @@ function Test-IsFileLockError {
 }
 
 function Invoke-WithFileLockRetry {
+    <#
+    .SYNOPSIS
+        Runs an action and retries once when a file lock conflict is detected.
+    .DESCRIPTION
+        On lock-related failures, performs the same process cleanup used by build/test scripts
+        before retrying. When RepoRoot is provided, cleanup remains worktree-aware.
+    .PARAMETER Action
+        Script block to execute.
+    .PARAMETER Context
+        Friendly text used in retry warnings.
+    .PARAMETER IncludeOmniSharp
+        Includes OmniSharp processes in cleanup candidates.
+    .PARAMETER RepoRoot
+        Optional repository root used to scope cleanup to the current worktree.
+    .PARAMETER MaxAttempts
+        Number of attempts (default 2).
+    #>
     param(
         [Parameter(Mandatory)][ScriptBlock]$Action,
         [Parameter(Mandatory)][string]$Context,
         [switch]$IncludeOmniSharp,
+        [string]$RepoRoot,
         [int]$MaxAttempts = 2
     )
 
@@ -173,7 +344,7 @@ function Invoke-WithFileLockRetry {
             if ($attempt -lt $MaxAttempts -and (Test-IsFileLockError -ErrorRecord $_)) {
                 $nextAttempt = $attempt + 1
                 Write-Host "[WARN] $Context hit a file lock. Cleaning and retrying (attempt $nextAttempt of $MaxAttempts)..." -ForegroundColor Yellow
-                Stop-ConflictingProcesses -IncludeOmniSharp:$IncludeOmniSharp
+                Stop-ConflictingProcesses -IncludeOmniSharp:$IncludeOmniSharp -RepoRoot $RepoRoot
                 Start-Sleep -Seconds 2
                 $retry = $true
             }
@@ -321,6 +492,8 @@ Export-ModuleMember -Function @(
     'Get-CvtresDiagnostics',
     # Local functions
     'Stop-ConflictingProcesses',
+    'Enter-WorktreeLock',
+    'Exit-WorktreeLock',
     'Remove-StaleObjFolders',
     'Test-IsFileLockError',
     'Invoke-WithFileLockRetry'

ReadMe.md

@@ -49,6 +49,14 @@ FieldWorks uses the **MSBuild Traversal SDK** for declarative, dependency-ordere
 
 For detailed build instructions, see [.github/instructions/build.instructions.md](.github/instructions/build.instructions.md).
 
+### Concurrent worktree builds/tests
+
+`build.ps1` and `test.ps1` use worktree-aware process cleanup, so running scripted builds/tests in different git worktrees does not kill each other.
+
+Within a single worktree, builds and tests run one at a time: scripts acquire a worktree lock and fail fast if another scripted workflow is active.
+
+You can tag lock ownership for diagnostics with `FW_BUILD_STARTED_BY=user|agent` (or `-StartedBy user|agent`).
+
 ## Building Installers (WiX 3 default, WiX 6 opt-in)
 
 Installer builds include the additional utilities (UnicodeCharEditor, LCMBrowser, MigrateSqlDbs, etc.).

build.ps1

@@ -42,7 +42,8 @@
 	is written next to the built executable. Useful for crash investigation.
 
 .PARAMETER NodeReuse
-	Enables or disables MSBuild node reuse (/nr). Default is true.
+	Enables or disables MSBuild node reuse (/nr). Default is false to improve
+	worktree isolation for concurrent local builds. Set to true to favor speed over isolation.
 
 .PARAMETER MsBuildArgs
 	Additional arguments to pass directly to MSBuild.
@@ -80,6 +81,10 @@
 .PARAMETER LogFile
 	Path to a file where the build output should be logged.
 
+.PARAMETER StartedBy
+	Optional actor label written to the worktree lock metadata (for example: user or agent).
+	Defaults to the FW_BUILD_STARTED_BY environment variable when set, otherwise 'unknown'.
+
 .PARAMETER TailLines
 	If specified, only displays the last N lines of output after the build completes.
 	Useful for CI/agent scenarios where you want to see recent output without piping.
@@ -120,7 +125,7 @@ param(
 	[switch]$BuildAdditionalApps,
 	[string]$Project = "FieldWorks.proj",
 	[string]$Verbosity = "minimal",
-	[bool]$NodeReuse = $true,
+	[bool]$NodeReuse = $false,
 	[string[]]$MsBuildArgs = @(),
 	[string]$LogFile,
 	[int]$TailLines,
@@ -134,11 +139,20 @@ param(
 	[switch]$SignInstaller,
 	[switch]$TraceCrashes,
 	[switch]$UseLocalLcm,
-	[string]$LocalLcmPath
+	[string]$LocalLcmPath,
+	[ValidateSet('user', 'agent', 'unknown')]
+	[string]$StartedBy = 'unknown'
 )
 
 $ErrorActionPreference = "Stop"
 
+if (-not $PSBoundParameters.ContainsKey('StartedBy') -and -not [string]::IsNullOrWhiteSpace($env:FW_BUILD_STARTED_BY)) {
+	$startedByFromEnv = $env:FW_BUILD_STARTED_BY.ToLowerInvariant()
+	if ($startedByFromEnv -in @('user', 'agent', 'unknown')) {
+		$StartedBy = $startedByFromEnv
+	}
+}
+
 if ($Configuration -like "--*") {
 	if ($Configuration -eq "--TraceCrashes" -and -not $TraceCrashes) {
 		$TraceCrashes = $true
@@ -190,7 +204,10 @@ if (-not (Test-Path $helpersPath)) {
 }
 Import-Module $helpersPath -Force
 
-Stop-ConflictingProcesses -IncludeOmniSharp
+$worktreeLock = Enter-WorktreeLock -RepoRoot $PSScriptRoot -Context "FieldWorks build" -StartedBy $StartedBy
+
+# Worktree-aware cleanup: only stop conflicting processes related to this repo root.
+Stop-ConflictingProcesses -IncludeOmniSharp -RepoRoot $PSScriptRoot
 
 $fwTasksSourcePath = Join-Path $PSScriptRoot "BuildTools/FwBuildTasks/$Configuration/FwBuildTasks.dll"
 $fwTasksDropPath = Join-Path $PSScriptRoot "BuildTools/FwBuildTasks/$Configuration/FwBuildTasks.dll"
@@ -268,7 +285,7 @@ function Get-BuildStampPath {
 }
 
 try {
-	Invoke-WithFileLockRetry -Context "FieldWorks build" -IncludeOmniSharp -Action {
+	Invoke-WithFileLockRetry -Context "FieldWorks build" -IncludeOmniSharp -RepoRoot $PSScriptRoot -Action {
 		# Initialize Visual Studio Developer environment
 		Initialize-VsDevEnvironment
 		Test-CvtresCompatibility
@@ -528,6 +545,7 @@ try {
 		Write-Host "Running tests..." -ForegroundColor Cyan
 
 		$testArgs = @("-Configuration", $Configuration, "-NoBuild")
+		$testArgs += "-SkipWorktreeLock"
 		if ($TestFilter) {
 			$testArgs += @("-TestFilter", $TestFilter)
 		}
@@ -543,6 +561,7 @@ try {
 finally {
 	# Kill any lingering build processes that might hold file locks
 	Stop-ConflictingProcesses @cleanupArgs
+	Exit-WorktreeLock -LockHandle $worktreeLock
 }
 
 if ($testExitCode -ne 0) {