From 1c4e5763f2e4e3d9f8ec4fb8d7cc326c9f85641e Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Mon, 24 Aug 2026 09:32:48 +0100 Subject: [PATCH 1/5] feat(resolve-dependencies): report the resolved ref on every invocation _selection.txt is deleted on every invocation and was written only when a clone was created, so a second invocation in the same job produced no selection table and said nothing about which refs it was building. ci-serialisation's baseline leg is exactly that case: its dependency refs were invisible. Records the selected ref beside the clones, in a flat file so the junction loop does not link it as a dependency, and reads it back on reuse. A detached HEAD cannot name the branch it came from, so the marker is the only way to report it. Deliberately separate from the resolution fix that follows: this is the diagnostic that makes the defect visible, and it stands on its own if that fix is reverted. --- .../scripts/Resolve-DependencyGraph.ps1 | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 index cb5399c..76f3f8e 100644 --- a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 +++ b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 @@ -16,6 +16,13 @@ $shaFile = Join-Path $depsDir "_shas.txt" $orderOut = Join-Path $depsDir "_order.txt" $selectFile = Join-Path $depsDir "_selection.txt" +# Which ref each clone was last checked out to, kept beside the clones rather than in deps/ +# because the action truncates _shas.txt and deletes _selection.txt on every invocation while +# the clone root persists for the whole job. A flat file, not a directory: the junction loop at +# the end of the calling action enumerates directories under the clone root and would otherwise +# link this into the workspace parent as if it were a dependency. +$refMarkerFile = Join-Path $cloneRoot "_selected-refs.txt" + New-Item -ItemType Directory -Force -Path $cloneRoot | Out-Null if (Test-Path $selectFile) { Remove-Item $selectFile -Force } @@ -83,6 +90,28 @@ function Get-FolderName([string]$ownerRepo) { return $name } +# A detached HEAD cannot report the branch it came from, so the ref each clone was checked out +# to is recorded here and read back on a later invocation. Without it a reused clone can only be +# described by its SHA, which does not tell a reader whether the ref was the one this leg asked +# for. See the reporting note on the reuse path in Clone-And-Checkout. +function Set-SelectedRefMarker([string]$name, [string]$ref) { + $kept = @() + if (Test-Path $refMarkerFile) { + $kept = @(Get-Content $refMarkerFile | + Where-Object { $_ -notmatch "^$([regex]::Escape($name))\|" }) + } + ($kept + "$name|$ref") | Set-Content -Path $refMarkerFile -Encoding utf8 +} + +function Get-SelectedRefMarker([string]$name) { + if (-not (Test-Path $refMarkerFile)) { return $null } + $line = Get-Content $refMarkerFile | + Where-Object { $_ -match "^$([regex]::Escape($name))\|" } | + Select-Object -First 1 + if (-not $line) { return $null } + return $line.Split('|', 2)[1] +} + function Clone-And-Checkout([string]$ownerRepo, [string]$ref) { $name = Get-FolderName $ownerRepo @@ -140,6 +169,7 @@ function Clone-And-Checkout([string]$ownerRepo, [string]$ref) { $sha = (git rev-parse HEAD).Trim() Add-Content -Path $shaFile -Value "$ownerRepo $sha" Add-Content -Path $selectFile -Value "$ownerRepo|$name|$selectedRef|$sha" + Set-SelectedRefMarker $name $selectedRef } finally { Pop-Location @@ -162,6 +192,18 @@ function Clone-And-Checkout([string]$ownerRepo, [string]$ref) { try { $sha = (git rev-parse HEAD).Trim() Add-Content -Path $shaFile -Value "$ownerRepo $sha" + + # Report the reuse rather than staying silent about it. _selection.txt is deleted on + # every invocation and was previously written only when a clone was created, so a + # second invocation in the same job produced no selection table at all and its log + # said nothing about which refs it was actually building. That silence is what let + # ci-serialisation's baseline leg look like it had resolved its own dependencies. + # The ref comes from the marker because HEAD here is detached at FETCH_HEAD and + # cannot name the branch it came from. + $reusedRef = Get-SelectedRefMarker $name + if (-not $reusedRef) { $reusedRef = "(unrecorded)" } + Add-Content -Path $selectFile -Value "$ownerRepo|$name|$reusedRef (reused)|$sha" + Write-Host "Dependency reused: $ownerRepo -> $reusedRef @ $($sha.Substring(0,7)) (clone already present, not re-resolved)" } finally { Pop-Location From 734f02348adffb961f0044ca6be86966d225a912 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Mon, 24 Aug 2026 09:08:49 +0100 Subject: [PATCH 2/5] test(resolve-dependencies): demonstrate the ci-serialisation baseline reuse ci-serialisation invokes resolve-dependencies twice in one job, once for the pull request branch and once for the base it is compared against. Pester tests establishing that the baseline leg is built against the branch's dependency code, and that the two causes are independent: the already-cloned shortcut at Resolve-DependencyGraph.ps1:91, and PR_BRANCH being sourced from the event payload with no per-invocation override. Hermetic: a local bare repo via git insteadOf, no network and no BHoM. An end-to-end reproduction needs a subject repo and a dependency in the same organisation carrying a branch of the same name, which would mean pushing branches to repositories that are in use. Demonstration only. Assertions describe current behaviour, and those expected to invert once this is fixed name the value they should then read. No production code touched. --- ...ve-DependencyGraph.BaselineReuse.Tests.ps1 | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 .github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 diff --git a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 new file mode 100644 index 0000000..3456d39 --- /dev/null +++ b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 @@ -0,0 +1,261 @@ +# Resolve-DependencyGraph.BaselineReuse.Tests.ps1 +# +# ci-serialisation invokes resolve-dependencies twice in one job: once for the pull request +# branch and once for the base it is compared against. The second invocation reuses the first +# invocation's clones without re-resolving them, so both legs are built against the BRANCH's +# dependency code. A regression introduced on the dependency side then appears in both legs, +# compares equal, and the check passes. +# +# Written as a DEMONSTRATION, not a specification. Every assertion here describes what the +# resolver does today. Fixing it is behaviour-changing and not yet scoped, so nothing here +# asserts a preferred behaviour. Assertions expected to invert once it is fixed say so, and +# name the value they should then read. +# +# Why a hermetic test rather than a live pair of repositories. Reproducing this end to end +# needs a subject repo whose dependencies.txt names a dependency in the same organisation, +# plus a branch of the same name on both. No such pair exists that is free to experiment on, +# and making one would mean pushing branches to repositories that are in use. A local bare +# repo reaches the same code path with no network. +# +# What it establishes, in order: +# 1. On a fresh clone root, the resolver honours PR_BRANCH when that branch exists on the +# dependency (the intended cross-repo feature). +# 2. Invoked a second time against the SAME clone root, it does not re-check-out anything, +# and records the same SHA. This is the "already cloned" path at +# Resolve-DependencyGraph.ps1:91 and it is what ci-serialisation's baseline leg hits. +# 3. The resolver CAN land on the base branch when asked to, so the defect is not that it +# cannot; it is that ci-serialisation never asks. PR_BRANCH is sourced from +# github.event.pull_request.head.ref in resolve-dependencies/action.yml:132, which is +# constant for the whole job and has no per-invocation override. +# 4. Even if a caller COULD ask for the base branch on the second invocation, the +# already-cloned shortcut would ignore it. Two independent causes, so a fix addressing +# only one of them does not work. This is the assertion that matters most. +# +# Run locally: pwsh -Command "Invoke-Pester .github/scripts/tests -Output Detailed" +# Run in CI: lint-workflows.yml, the powershell-tests job. + +BeforeAll { + $script:repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '../../..')).Path + $script:resolver = Join-Path $repoRoot '.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1' + $script:sandbox = Join-Path ([IO.Path]::GetTempPath()) ("depgraph-" + [Guid]::NewGuid().ToString('N')) + + # The resolver hard-codes https://github.com//.git, so the only way to point + # it at a local fixture is git's insteadOf rewrite. Same mechanism the resolver itself + # uses for tokens. Removed in AfterAll; nothing else in the powershell-tests job clones + # from github.com after this file runs. + $script:remoteRoot = Join-Path $sandbox 'remotes' + New-Item -ItemType Directory -Force -Path $remoteRoot | Out-Null + $script:insteadOfKey = 'url.file:///' + ($remoteRoot -replace '\\', '/') + '/.insteadOf' + git config --global $insteadOfKey 'https://github.com/' + + # The resolver appends a markdown table to the step summary when it clones. Redirect it so + # a test run does not write into the real job summary. + $script:savedSummary = $env:GITHUB_STEP_SUMMARY + $env:GITHUB_STEP_SUMMARY = Join-Path $sandbox 'summary.md' + + function New-FixtureRemote { + # A bare repo with two branches whose tip contents differ, standing in for a BHoM + # dependency that carries a change on a feature branch. + param([string]$OwnerRepo, [string]$Prefer) + + $work = Join-Path $sandbox ('work-' + ($OwnerRepo -replace '/', '-')) + $bare = Join-Path $remoteRoot "$OwnerRepo.git" + New-Item -ItemType Directory -Force -Path (Split-Path $bare) | Out-Null + + git init -q --bare $bare + git init -q $work + Push-Location $work + try { + git config user.email 't@t'; git config user.name 't' + git symbolic-ref HEAD refs/heads/develop + + Set-Content -Path 'Value.cs' -Value '// base' -Encoding utf8 + git add -A; git commit -q -m 'base' + $baseSha = (git rev-parse HEAD).Trim() + + git checkout -q -b $Prefer + Set-Content -Path 'Value.cs' -Value '// branch change, serialisation-affecting' -Encoding utf8 + git add -A; git commit -q -m 'branch' + $branchSha = (git rev-parse HEAD).Trim() + + git remote add origin $bare + git push -q origin develop $Prefer + # HEAD on the bare repo decides the remote-default fallback. + git --git-dir=$bare symbolic-ref HEAD refs/heads/develop + } + finally { Pop-Location } + + return @{ Base = $baseSha; Branch = $branchSha } + } + + function Invoke-Resolver { + # Reproduces exactly what resolve-dependencies/action.yml does around the script: + # create deps/, truncate _shas.txt (New-Item -ItemType File -Force at :83), set + # PR_BRANCH and BASE_BRANCH from the event, then invoke from the workspace root. + param( + [string]$Workspace, + [string]$CloneRoot, + [AllowNull()][string]$Prefer, + [string]$Fallback + ) + + New-Item -ItemType Directory -Force -Path (Join-Path $Workspace 'deps') | Out-Null + New-Item -ItemType File -Force -Path (Join-Path $Workspace 'deps/_shas.txt') | Out-Null + + $env:PR_BRANCH = $Prefer + $env:BASE_BRANCH = $Fallback + $env:DEP_TOKEN = '' + + Push-Location $Workspace + try { + & $resolver -DepsFile 'dependencies.txt' -Mode 'caller' -Seeds '' ` + -AdditionalSeeds '' -CloneRoot $CloneRoot | Out-Null + } + finally { Pop-Location } + } + + function Get-CheckedOutSha { + param([string]$CloneRoot, [string]$Name) + Push-Location (Join-Path $CloneRoot $Name) + try { return (git rev-parse HEAD).Trim() } finally { Pop-Location } + } + + function Get-RecordedSha { + param([string]$Workspace, [string]$OwnerRepo) + $line = Get-Content (Join-Path $Workspace 'deps/_shas.txt') | + Where-Object { $_ -like "$OwnerRepo *" } | Select-Object -First 1 + if (-not $line) { return $null } + return $line.Split(' ')[1] + } + + function New-Workspace { + param([string]$Name, [string]$Dependency) + $ws = Join-Path $sandbox $Name + New-Item -ItemType Directory -Force -Path $ws | Out-Null + Set-Content -Path (Join-Path $ws 'dependencies.txt') -Value $Dependency -Encoding utf8 + return $ws + } +} + +AfterAll { + git config --global --unset $insteadOfKey 2>$null + $env:GITHUB_STEP_SUMMARY = $savedSummary + Remove-Item $sandbox -Recurse -Force -ErrorAction SilentlyContinue +} + +Describe 'resolve-dependencies: ref selection across repeat invocations in one job' { + + BeforeAll { + $script:dep = 'Fake/Dep' + $script:prefer = 'feature/dependency-side-change' + $script:shas = New-FixtureRemote -OwnerRepo $dep -Prefer $prefer + } + + Context 'the preferred branch is honoured when the dependency carries it' { + + It 'checks the dependency out at the branch tip, not the base tip' { + $ws = New-Workspace -Name 'prefer-ws' -Dependency $dep + $root = Join-Path $sandbox 'prefer-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Branch + Get-RecordedSha -Workspace $ws -OwnerRepo $dep | Should -Be $shas.Branch + } + } + + Context 'a second invocation reuses the first invocation clone' { + + It 'does not move the dependency off the branch tip on a second invocation' { + $ws = New-Workspace -Name 'same-ref-ws' -Dependency $dep + $root = Join-Path $sandbox 'same-ref-clones' + + # Branch leg. + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $afterBranchLeg = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' + + # Baseline leg. PR_BRANCH is unchanged because resolve-dependencies sources it + # from the event payload, which does not vary within a job. ci-serialisation + # clears ProgramData\BHoM\Assemblies between the legs but never the clone root, + # so this is what its second resolve sees. + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $afterBaselineLeg = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' + + $afterBranchLeg | Should -Be $shas.Branch + # Should read $shas.Base once the baseline leg re-resolves; today it does not. + $afterBaselineLeg | Should -Be $shas.Branch + $afterBaselineLeg | Should -Be $afterBranchLeg + } + + It 'records the same SHA both times, so the assembly cache key is identical' { + $ws = New-Workspace -Name 'same-ref-sha-ws' -Dependency $dep + $root = Join-Path $sandbox 'same-ref-sha-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $first = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + # The depsasm- cache key is a SHA-256 over the sorted owner/repo@sha set + # (resolve-dependencies/action.yml:145-164). Identical recorded SHAs therefore + # mean an identical key, which is why the baseline leg restores the branch leg's + # assemblies instead of building its own: the key misses on the branch leg, which + # builds and saves it, then hits on the baseline leg, which clones nothing and + # builds nothing. + $second | Should -Be $first + # Should read $shas.Base once the two legs resolve independently. + $second | Should -Be $shas.Branch + } + } + + Context 'the resolver can reach the base branch, but was never asked to' { + + It 'lands on the base tip when PR_BRANCH does not exist on the dependency' { + $ws = New-Workspace -Name 'base-reach-ws' -Dependency $dep + $root = Join-Path $sandbox 'base-reach-clones' + + # The common case: the PR branch name exists only on the subject repo, so + # $Prefer misses and $Fallback wins. This is why the defect was never observed + # firing: it needs a cross-repo branch pair, and most PRs are not one. + Invoke-Resolver -Workspace $ws -CloneRoot $root ` + -Prefer 'branch/that/exists/nowhere' -Fallback 'develop' + + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Base + } + + It 'lands on the base tip on a fresh root when asked for the base explicitly' { + $ws = New-Workspace -Name 'base-explicit-ws' -Dependency $dep + $root = Join-Path $sandbox 'base-explicit-clones' + + # Proves the capability exists. resolve-dependencies simply exposes no input + # that would let ci-serialisation's baseline leg request it. + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Base + } + } + + Context 'the two causes are independent' { + + It 'ignores an explicit base-branch request when the clone is already present' { + $ws = New-Workspace -Name 'reresolve-ws' -Dependency $dep + $root = Join-Path $sandbox 'reresolve-clones' + + # Branch leg, as normal. + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Branch + + # Now ask for the base branch on the second invocation, i.e. pretend the caller + # had been given the per-leg parameter it currently lacks. The already-cloned + # path at :91 short-circuits before any ref resolution, so the request is ignored. + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + # THE ASSERTION THAT MATTERS: adding a per-leg branch parameter alone would not + # fix this; the clone reuse has to be addressed too. Both of these should read + # $shas.Base once it is. + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Branch + Get-RecordedSha -Workspace $ws -OwnerRepo $dep | Should -Be $shas.Branch + } + } +} From f2af2eb23750e5d265be222760aa41a7817308da Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Mon, 24 Aug 2026 09:36:12 +0100 Subject: [PATCH 3/5] fix(resolve-dependencies): resolve dependency refs on every invocation ci-serialisation's baseline leg was built against the branch leg's dependency code, so a regression introduced on the dependency side appeared in both legs, compared equal, and the check passed. Two independent causes, both addressed, because the demonstration test proved either alone is insufficient: Clone-And-Checkout short-circuited on an existing clone and only re-recorded its SHA. The clone is now conditional and the ref resolution is not, which is BHoMBot's shape (LoadDependencies cloned if absent then always checked out). PR_BRANCH came from the event payload with no override, so the baseline leg could not ask for the base branch. resolve-dependencies now takes a prefer_branch input and ci-serialisation's baseline leg passes base_ref. Cost is one shallow fetch per dependency per additional invocation, paid only by ci-serialisation. Single-invocation callers are unaffected, which the unchanged control tests assert. This does not make ci-serialisation a required check. --- .github/actions/ci-serialisation/action.yml | 8 + .../actions/resolve-dependencies/action.yml | 20 +- .../scripts/Resolve-DependencyGraph.ps1 | 171 ++++++++-------- ...ve-DependencyGraph.BaselineReuse.Tests.ps1 | 189 ++++++++++++------ 4 files changed, 248 insertions(+), 140 deletions(-) diff --git a/.github/actions/ci-serialisation/action.yml b/.github/actions/ci-serialisation/action.yml index a21c23c..e3036c8 100644 --- a/.github/actions/ci-serialisation/action.yml +++ b/.github/actions/ci-serialisation/action.yml @@ -198,6 +198,13 @@ runs: git checkout ${{ inputs.base_ref }} Remove-Item "C:\ProgramData\BHoM\Assemblies\*" -Recurse -Force -ErrorAction SilentlyContinue + # prefer_branch is what makes this a baseline. Without it the head branch is preferred on + # both legs, because resolve-dependencies takes PR_BRANCH from the event payload and that + # does not vary within a job. Every dependency carrying a branch of the same name as the + # pull request was therefore resolved to it twice, so the two legs shared dependency code + # and a regression on the dependency side cancelled out. Passing the base branch here, and + # re-resolving already-present clones in the resolver, are both required: either alone + # leaves the baseline on the branch leg's refs. - name: Resolve dependencies (baseline) if: steps.changed.outputs.count != '0' && steps.branch_run.outputs.status == 'Error' uses: BHoM/CI_Toolkit/.github/actions/resolve-dependencies@develop @@ -206,6 +213,7 @@ runs: dotnet_version: ${{ inputs.dotnet_version }} configuration: Release token: ${{ steps.dep-token.outputs.token }} + prefer_branch: ${{ inputs.base_ref }} additional_seeds: | ${{ inputs.test_toolkit_repo }} ${{ inputs.serialisation_engine_repo }} diff --git a/.github/actions/resolve-dependencies/action.yml b/.github/actions/resolve-dependencies/action.yml index 4fea0d5..7cf75f8 100644 --- a/.github/actions/resolve-dependencies/action.yml +++ b/.github/actions/resolve-dependencies/action.yml @@ -53,6 +53,21 @@ inputs: required: false default: "" + prefer_branch: + description: | + Branch to try first on every dependency, overriding the pull request's head branch. + + Empty (the default) keeps the existing behaviour: prefer the PR head branch, which is + what a branch-leg build wants and is how cross-repo pull request pairs resolve. + + Set this when a job resolves dependencies more than once and the passes mean different + things. ci-serialisation's baseline leg passes the base branch, because without it the + head branch is preferred on both legs: PR_BRANCH comes from the event payload, which + does not vary within a job, so the baseline leg had no way to ask for anything else. + An explicit owner/repo@ref in dependencies.txt still wins over this. + required: false + default: "" + runs: using: "composite" @@ -129,7 +144,10 @@ runs: - name: Resolve dependency graph shell: pwsh env: - PR_BRANCH: ${{ github.event.pull_request.head.ref }} + # prefer_branch wins when supplied. The event payload is the default because it is + # right for every caller that resolves once; it is not overridable from inside the + # script, which is why the baseline leg needed an input rather than a code change. + PR_BRANCH: ${{ inputs.prefer_branch || github.event.pull_request.head.ref }} BASE_BRANCH: ${{ github.event.pull_request.base.ref }} DEP_TOKEN: ${{ inputs.token }} run: | diff --git a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 index 76f3f8e..be76581 100644 --- a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 +++ b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 @@ -117,97 +117,102 @@ function Clone-And-Checkout([string]$ownerRepo, [string]$ref) { $name = Get-FolderName $ownerRepo $path = Join-Path $cloneRoot $name - if (-not (Test-Path (Join-Path $path ".git"))) { - + # Clone if absent, then resolve the ref UNCONDITIONALLY. + # + # These two used to be one conditional: a clone that already existed was left on whatever + # ref a previous invocation put it on, and only its SHA was re-recorded. ci-serialisation + # invokes this action twice in one job, so its baseline leg inherited the branch leg's + # dependency refs, built the base-branch subject against branch-branch dependencies, and + # compared two legs that shared the same dependency code. A regression introduced on the + # dependency side appeared in both and cancelled out. + # + # This is BHoMBot's shape, deliberately. LoadDependencies (CodeBuild_Engine) cloned if + # absent and then always called CheckoutBranch with the branch as an explicit argument, and + # ResetBuiltRepos() cleared the memo that would otherwise have skipped the re-checkout. The + # divergence from BHoMBot is that the memo has no equivalent here and needs none: there is + # no build-dedup cache in this script, so nothing suppresses the second resolution. + # + # Cost is one shallow fetch per dependency per additional invocation, which only + # ci-serialisation pays. Not optimised away by comparing the remote SHA first: that adds a + # branch to reason about in exchange for saving a fetch nothing is waiting on. + $freshClone = -not (Test-Path (Join-Path $path ".git")) + if ($freshClone) { git clone "https://github.com/$ownerRepo.git" $path --no-tags --depth 1 | Out-Null + } - $selectedRef = $null - Push-Location $path - try { - $used = $false - - if ($ref) { - $hasHead = git ls-remote --heads origin $ref - $hasTag = git ls-remote --tags origin $ref - if ($hasHead -or $hasTag) { - git fetch origin $ref --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$ref)" } - $selectedRef = $ref - $used = $true - } else { - Write-Warning "Explicit ref '$ref' not found on '$ownerRepo' — falling back." - } + $previousRef = if ($freshClone) { $null } else { Get-SelectedRefMarker $name } + + $selectedRef = $null + Push-Location $path + try { + $used = $false + + if ($ref) { + $hasHead = git ls-remote --heads origin $ref + $hasTag = git ls-remote --tags origin $ref + if ($hasHead -or $hasTag) { + git fetch origin $ref --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$ref)" } + $selectedRef = $ref + $used = $true + } else { + Write-Warning "Explicit ref '$ref' not found on '$ownerRepo' — falling back." } + } - if (-not $used) { - $hasPrefer = if ($Prefer) { git ls-remote --heads origin $Prefer } else { $null } - $hasFallback = git ls-remote --heads origin $Fallback - if ($hasPrefer) { - git fetch origin $Prefer --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Prefer)" } - $selectedRef = $Prefer - } elseif ($hasFallback) { - git fetch origin $Fallback --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Fallback)" } - $selectedRef = $Fallback - } else { - # Neither PR nor base branch exists on this dep; fall back to remote default. - git fetch origin HEAD --depth 1 | Out-Null - git checkout -q FETCH_HEAD - if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (remote default)" } - $defaultRef = (git ls-remote --symref origin HEAD | - Select-String 'ref: refs/heads/(\S+)\s+HEAD' | - ForEach-Object { $_.Matches[0].Groups[1].Value } | - Select-Object -First 1) - $selectedRef = if ($defaultRef) { $defaultRef } else { "(remote default)" } - } + if (-not $used) { + $hasPrefer = if ($Prefer) { git ls-remote --heads origin $Prefer } else { $null } + $hasFallback = git ls-remote --heads origin $Fallback + if ($hasPrefer) { + git fetch origin $Prefer --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Prefer)" } + $selectedRef = $Prefer + } elseif ($hasFallback) { + git fetch origin $Fallback --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (ref=$Fallback)" } + $selectedRef = $Fallback + } else { + # Neither PR nor base branch exists on this dep; fall back to remote default. + git fetch origin HEAD --depth 1 | Out-Null + git checkout -q FETCH_HEAD + if ($LASTEXITCODE -ne 0) { throw "git checkout FETCH_HEAD failed for '$ownerRepo' (remote default)" } + $defaultRef = (git ls-remote --symref origin HEAD | + Select-String 'ref: refs/heads/(\S+)\s+HEAD' | + ForEach-Object { $_.Matches[0].Groups[1].Value } | + Select-Object -First 1) + $selectedRef = if ($defaultRef) { $defaultRef } else { "(remote default)" } } - - $sha = (git rev-parse HEAD).Trim() - Add-Content -Path $shaFile -Value "$ownerRepo $sha" - Add-Content -Path $selectFile -Value "$ownerRepo|$name|$selectedRef|$sha" - Set-SelectedRefMarker $name $selectedRef - } - finally { - Pop-Location } - # Write-Host, not ::notice. A ::notice with no file= becomes a check-run - # annotation attributed to ".github" at the log line number, and there is one - # per dependency: nine on a typical ci-build run, competing with the caller's - # own diagnostics for GitHub's per-step annotation cap. The same information - # already reaches the job summary as a table at the end of this script, which - # is where a reader looks for it. - Write-Host "Dependency checkout: $ownerRepo -> $selectedRef @ $($sha.Substring(0,7))" + # _shas.txt is reset between invocations, so this must be written every time, not only + # on a fresh clone. Without it the cache-key computation produces an empty keypart, + # skipping both the assembly cache restore and the dep build, leaving + # C:\ProgramData\BHoM\Assemblies empty. + $sha = (git rev-parse HEAD).Trim() + Add-Content -Path $shaFile -Value "$ownerRepo $sha" + Add-Content -Path $selectFile -Value "$ownerRepo|$name|$selectedRef|$sha" + Set-SelectedRefMarker $name $selectedRef } - else { - # Repo already cloned (e.g. baseline run re-uses branch-build clones). - # _shas.txt is reset between invocations, so write the SHA even though we skip re-cloning. - # Without this the cache-key computation produces an empty keypart, skipping both the - # assembly cache restore and the dep build, leaving C:\ProgramData\BHoM\Assemblies empty. - Push-Location $path - try { - $sha = (git rev-parse HEAD).Trim() - Add-Content -Path $shaFile -Value "$ownerRepo $sha" - - # Report the reuse rather than staying silent about it. _selection.txt is deleted on - # every invocation and was previously written only when a clone was created, so a - # second invocation in the same job produced no selection table at all and its log - # said nothing about which refs it was actually building. That silence is what let - # ci-serialisation's baseline leg look like it had resolved its own dependencies. - # The ref comes from the marker because HEAD here is detached at FETCH_HEAD and - # cannot name the branch it came from. - $reusedRef = Get-SelectedRefMarker $name - if (-not $reusedRef) { $reusedRef = "(unrecorded)" } - Add-Content -Path $selectFile -Value "$ownerRepo|$name|$reusedRef (reused)|$sha" - Write-Host "Dependency reused: $ownerRepo -> $reusedRef @ $($sha.Substring(0,7)) (clone already present, not re-resolved)" - } - finally { - Pop-Location - } + finally { + Pop-Location + } + + # Write-Host, not ::notice. A ::notice with no file= becomes a check-run + # annotation attributed to ".github" at the log line number, and there is one + # per dependency: nine on a typical ci-build run, competing with the caller's + # own diagnostics for GitHub's per-step annotation cap. The same information + # already reaches the job summary as a table at the end of this script, which + # is where a reader looks for it. + # + # A re-resolution that moved the clone names both refs, because "which ref is this leg + # building" is the question ci-serialisation's baseline leg could not previously answer. + if ($previousRef -and $previousRef -ne $selectedRef) { + Write-Host "Dependency re-resolved: $ownerRepo -> $selectedRef @ $($sha.Substring(0,7)) (was $previousRef)" + } else { + Write-Host "Dependency checkout: $ownerRepo -> $selectedRef @ $($sha.Substring(0,7))" } $nameMap[$ownerRepo] = $name diff --git a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 index 3456d39..43047eb 100644 --- a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 +++ b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 @@ -1,35 +1,43 @@ # Resolve-DependencyGraph.BaselineReuse.Tests.ps1 # # ci-serialisation invokes resolve-dependencies twice in one job: once for the pull request -# branch and once for the base it is compared against. The second invocation reuses the first -# invocation's clones without re-resolving them, so both legs are built against the BRANCH's -# dependency code. A regression introduced on the dependency side then appears in both legs, -# compares equal, and the check passes. +# branch and once for the base it is compared against. Before this change the second invocation +# reused the first invocation's clones without re-resolving them, so both legs were built against +# the BRANCH's dependency code. A regression introduced on the dependency side then appeared in +# both legs, compared equal, and the check passed. # -# Written as a DEMONSTRATION, not a specification. Every assertion here describes what the -# resolver does today. Fixing it is behaviour-changing and not yet scoped, so nothing here -# asserts a preferred behaviour. Assertions expected to invert once it is fixed say so, and -# name the value they should then read. +# These began as a demonstration of that defect and are now the regression suite for its fix. +# The assertions under "re-resolving an existing clone" and "reporting" originally asserted the +# broken behaviour and have been inverted. The three preceding Contexts were controls and are +# unchanged, which is what makes them useful: they show the fix did not alter resolution for +# callers that invoke the action once, which is every check except ci-serialisation. # -# Why a hermetic test rather than a live pair of repositories. Reproducing this end to end -# needs a subject repo whose dependencies.txt names a dependency in the same organisation, -# plus a branch of the same name on both. No such pair exists that is free to experiment on, -# and making one would mean pushing branches to repositories that are in use. A local bare -# repo reaches the same code path with no network. +# This change does not make ci-serialisation a required check. +# +# Why a hermetic test rather than a live pair of repositories. Reproducing this end to end needs +# a subject repo whose dependencies.txt names a dependency in the same organisation, plus a +# branch of the same name on both. No such pair exists that is free to experiment on, and making +# one would mean pushing branches to repositories that are in use. A local bare repo reaches the +# same code path with no network. # # What it establishes, in order: # 1. On a fresh clone root, the resolver honours PR_BRANCH when that branch exists on the -# dependency (the intended cross-repo feature). -# 2. Invoked a second time against the SAME clone root, it does not re-check-out anything, -# and records the same SHA. This is the "already cloned" path at -# Resolve-DependencyGraph.ps1:91 and it is what ci-serialisation's baseline leg hits. -# 3. The resolver CAN land on the base branch when asked to, so the defect is not that it -# cannot; it is that ci-serialisation never asks. PR_BRANCH is sourced from -# github.event.pull_request.head.ref in resolve-dependencies/action.yml:132, which is -# constant for the whole job and has no per-invocation override. -# 4. Even if a caller COULD ask for the base branch on the second invocation, the -# already-cloned shortcut would ignore it. Two independent causes, so a fix addressing -# only one of them does not work. This is the assertion that matters most. +# dependency. That is the intended cross-repo feature and the fix must not disturb it. +# 2. Two invocations that both ask for the same ref agree: the checkout stays where it is and +# the recorded SHA does not change. A control on the always-re-resolve change, since an +# unstable repeat resolution would show up here first. +# 3. The resolver can land on the base branch when asked to, so the defect was never that it +# could not. PR_BRANCH was taken from github.event.pull_request.head.ref in +# resolve-dependencies/action.yml, which is constant for a whole job and had no +# per-invocation override, so the baseline leg had no way to ask for anything else. +# 4. An existing clone is re-resolved, so an explicit base-branch request is honoured, the +# working tree moves and not just the recorded SHA, and the two legs produce different +# cache-key inputs. These assertions originally read the other way round, which is what +# showed both causes had to be addressed together: adding the input alone would have been +# ignored by the shortcut that skipped resolution on an already-present clone. +# 5. The selection table and the ref marker are written on every invocation, not only when a +# clone is created, and a repeat resolution is annotated according to who asked for the +# ref it settled on. # # Run locally: pwsh -Command "Invoke-Pester .github/scripts/tests -Output Detailed" # Run in CI: lint-workflows.yml, the powershell-tests job. @@ -164,30 +172,27 @@ Describe 'resolve-dependencies: ref selection across repeat invocations in one j } } - Context 'a second invocation reuses the first invocation clone' { + Context 'a second invocation asking for the same ref is stable' { - It 'does not move the dependency off the branch tip on a second invocation' { + # Control, not a defect. Two invocations that both want the branch must agree. This + # guards the ordinary single-meaning case against the always-re-resolve change: if the + # fix had made repeat resolution unstable, this is what would catch it. + It 'stays on the branch tip when both invocations prefer the branch' { $ws = New-Workspace -Name 'same-ref-ws' -Dependency $dep $root = Join-Path $sandbox 'same-ref-clones' - # Branch leg. Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' - $afterBranchLeg = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' + $afterFirst = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' - # Baseline leg. PR_BRANCH is unchanged because resolve-dependencies sources it - # from the event payload, which does not vary within a job. ci-serialisation - # clears ProgramData\BHoM\Assemblies between the legs but never the clone root, - # so this is what its second resolve sees. Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' - $afterBaselineLeg = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' + $afterSecond = Get-CheckedOutSha -CloneRoot $root -Name 'Dep' - $afterBranchLeg | Should -Be $shas.Branch - # Should read $shas.Base once the baseline leg re-resolves; today it does not. - $afterBaselineLeg | Should -Be $shas.Branch - $afterBaselineLeg | Should -Be $afterBranchLeg + $afterFirst | Should -Be $shas.Branch + $afterSecond | Should -Be $shas.Branch + $afterSecond | Should -Be $afterFirst } - It 'records the same SHA both times, so the assembly cache key is identical' { + It 'records the same SHA when both invocations prefer the same ref' { $ws = New-Workspace -Name 'same-ref-sha-ws' -Dependency $dep $root = Join-Path $sandbox 'same-ref-sha-clones' @@ -197,14 +202,7 @@ Describe 'resolve-dependencies: ref selection across repeat invocations in one j Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' $second = Get-RecordedSha -Workspace $ws -OwnerRepo $dep - # The depsasm- cache key is a SHA-256 over the sorted owner/repo@sha set - # (resolve-dependencies/action.yml:145-164). Identical recorded SHAs therefore - # mean an identical key, which is why the baseline leg restores the branch leg's - # assemblies instead of building its own: the key misses on the branch leg, which - # builds and saves it, then hits on the baseline leg, which clones nothing and - # builds nothing. $second | Should -Be $first - # Should read $shas.Base once the two legs resolve independently. $second | Should -Be $shas.Branch } } @@ -236,26 +234,105 @@ Describe 'resolve-dependencies: ref selection across repeat invocations in one j } } - Context 'the two causes are independent' { + Context 're-resolving an existing clone, which is the fix' { - It 'ignores an explicit base-branch request when the clone is already present' { + # This is the assertion the whole change exists for. Before the fix it read + # Should -Be $shas.Branch: the already-cloned path short-circuited before any ref + # resolution, so an explicit base-branch request was ignored and parameterising the + # branch alone would not have helped. Both halves are now in place, so the second + # invocation lands where it was asked to. + It 'honours an explicit base-branch request when the clone is already present' { $ws = New-Workspace -Name 'reresolve-ws' -Dependency $dep $root = Join-Path $sandbox 'reresolve-clones' - # Branch leg, as normal. Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Branch - # Now ask for the base branch on the second invocation, i.e. pretend the caller - # had been given the per-leg parameter it currently lacks. The already-cloned - # path at :91 short-circuits before any ref resolution, so the request is ignored. + # What ci-serialisation's baseline leg now does, via prefer_branch: base_ref. Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' - # THE ASSERTION THAT MATTERS: adding a per-leg branch parameter alone would not - # fix this; the clone reuse has to be addressed too. Both of these should read - # $shas.Base once it is. - Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Branch - Get-RecordedSha -Workspace $ws -OwnerRepo $dep | Should -Be $shas.Branch + Get-CheckedOutSha -CloneRoot $root -Name 'Dep' | Should -Be $shas.Base + Get-RecordedSha -Workspace $ws -OwnerRepo $dep | Should -Be $shas.Base + } + + It 'moves the working tree, not just the recorded SHA' { + $ws = New-Workspace -Name 'reresolve-tree-ws' -Dependency $dep + $root = Join-Path $sandbox 'reresolve-tree-clones' + $file = Join-Path (Join-Path $root 'Dep') 'Value.cs' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + (Get-Content $file -Raw) | Should -Match 'branch change' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + # Bookkeeping alone would leave the branch's source on disk and the base's SHA in + # _shas.txt, which is worse than the original defect: the cache key would say + # baseline while the assemblies were still built from branch code. + (Get-Content $file -Raw) | Should -Match 'base' + (Get-Content $file -Raw) | Should -Not -Match 'branch change' + } + + It 'gives the two legs different cache-key inputs' { + $ws = New-Workspace -Name 'reresolve-key-ws' -Dependency $dep + $root = Join-Path $sandbox 'reresolve-key-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $branchLegSha = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + $baselineLegSha = Get-RecordedSha -Workspace $ws -OwnerRepo $dep + + # The depsasm- key is a SHA-256 over the sorted owner/repo@sha set + # (resolve-dependencies/action.yml). Differing recorded SHAs mean a differing key, + # so the baseline leg now misses the branch leg's assembly cache and builds its own + # closure. Identical SHAs across the legs were the observable signature of the + # defect: the key missed on the branch leg, which built and saved it, then hit on + # the baseline leg, which therefore restored the branch leg's assemblies rather + # than building its own. + $baselineLegSha | Should -Not -Be $branchLegSha + } + } + + Context 'reporting: the resolved ref is recorded on every invocation' { + + It 'writes a selection line on a reused clone, naming the ref it resolved to' { + $ws = New-Workspace -Name 'rep-ws' -Dependency $dep + $root = Join-Path $sandbox 'rep-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + + # _selection.txt is deleted at the start of every invocation, so this content is + # the second invocation's alone. It was empty before the reporting change, which + # is why the baseline leg's job summary had no dependency table. + $selection = Get-Content (Join-Path $ws 'deps/_selection.txt') + $selection | Should -Not -BeNullOrEmpty + ($selection -join "`n") | Should -Match ([regex]::Escape("$dep|Dep|develop|")) + } + + It 'updates the marker so a later invocation reports the current ref' { + $ws = New-Workspace -Name 'rep2-ws' -Dependency $dep + $root = Join-Path $sandbox 'rep2-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $marker = Join-Path $root '_selected-refs.txt' + (Get-Content $marker -Raw) | Should -Match ([regex]::Escape("Dep|$prefer")) + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' + (Get-Content $marker -Raw) | Should -Match ([regex]::Escape('Dep|develop')) + (Get-Content $marker -Raw) | Should -Not -Match ([regex]::Escape($prefer)) + } + + It 'keeps the marker file out of the directories the caller junctions' { + $ws = New-Workspace -Name 'rep3-ws' -Dependency $dep + $root = Join-Path $sandbox 'rep3-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + + # The calling action junctions every DIRECTORY under the clone root into the + # workspace parent. A marker stored as a directory would be linked in as though it + # were a dependency. + Get-ChildItem $root -Directory | ForEach-Object { $_.Name } | Should -Be @('Dep') } } } From ff064dc6c84910156bda68c4f4f58fad98249b16 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Mon, 24 Aug 2026 10:06:29 +0100 Subject: [PATCH 4/5] feat(resolve-dependencies): warn when a repeat resolution changed nothing Residual risk in the preceding fix: prefer_branch has one caller, and a second caller passing a ref that does not exist would get a baseline built on the branch leg's dependency code with nothing to say so. Warns when every already-present dependency stayed on the ref the previous invocation chose, and emits a notice naming the moves when it did not. Warning rather than error deliberately: same-refs is legitimate and common, it is what happens whenever the pull request branch exists on no dependency, so failing would red most pull requests. Only reachable on a repeat invocation, so single-invocation callers see neither line. --- .../scripts/Resolve-DependencyGraph.ps1 | 31 +++++++++++++ ...ve-DependencyGraph.BaselineReuse.Tests.ps1 | 44 ++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 index be76581..cd26a3f 100644 --- a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 +++ b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 @@ -70,6 +70,9 @@ $nameMap = @{} # owner/repo -> folder $pathMap = @{} # owner/repo -> path $folderUsed = @{} # folder name -> owner/repo, for collision detection +# What each dependency resolved to, and what it was on before, for the no-op warning at the end. +$resolutionLog = New-Object System.Collections.Generic.List[hashtable] + # Use insteadOf to inject the token at the git config level — keeps it out of command # arguments, process listings, and cloned repos' .git/config. Removed in a finally block # to avoid persisting the credential on self-hosted runners even when the script errors. @@ -215,6 +218,8 @@ function Clone-And-Checkout([string]$ownerRepo, [string]$ref) { Write-Host "Dependency checkout: $ownerRepo -> $selectedRef @ $($sha.Substring(0,7))" } + $resolutionLog.Add(@{ Key = $ownerRepo; Previous = $previousRef; Selected = $selectedRef }) | Out-Null + $nameMap[$ownerRepo] = $name $pathMap[$ownerRepo] = $path @@ -373,6 +378,32 @@ if (Test-Path $selectFile) { } } +# A repeat invocation that changed nothing is the shape of the defect this reporting exists for: +# two passes meant to mean different things resolving to the same dependency code, so a regression +# on the dependency side appears in both and cancels out when they are compared. Only reachable on +# a repeat invocation, because a first pass has nothing to compare against. +# +# WARNING, not an error, and it must stay that way. Same-refs is legitimate and common: it is +# what happens whenever the pull request's branch name exists on no dependency, which is most +# pull requests. Failing here would red every one of them. The point is that a reader of a +# baseline leg can see which case they are in without reconstructing it from two logs. +$repeated = @($resolutionLog | Where-Object { $_.Previous }) +if ($repeated.Count -gt 0) { + $moved = @($repeated | Where-Object { $_.Previous -ne $_.Selected }) + if ($moved.Count -eq 0) { + Write-Host ("::warning title=resolve-dependencies::Repeat resolution changed nothing: all " + + "$($repeated.Count) already-present dependenc(ies) stayed on the ref the previous " + + "invocation selected. Expected when the requested branch exists on no dependency. " + + "If this is a baseline pass that was meant to differ from its branch pass, the two " + + "are sharing dependency code and any regression in it cancels out. " + + "Check prefer_branch is set and names a branch that exists.") + } else { + Write-Host ("::notice title=resolve-dependencies::Repeat resolution moved " + + "$($moved.Count) of $($repeated.Count) already-present dependenc(ies): " + + (($moved | ForEach-Object { "$($_.Key) $($_.Previous)->$($_.Selected)" }) -join ', ')) + } +} + } finally { # Remove the credential rewrite — on self-hosted runners ~/.gitconfig persists between # jobs and would leak the short-lived token to subsequent jobs. The finally block ensures diff --git a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 index 43047eb..541bfbe 100644 --- a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 +++ b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 @@ -104,7 +104,10 @@ BeforeAll { [string]$Workspace, [string]$CloneRoot, [AllowNull()][string]$Prefer, - [string]$Fallback + [string]$Fallback, + # Return the script's host output instead of discarding it, for the tests that + # assert on the workflow-command lines it emits. + [switch]$Capture ) New-Item -ItemType Directory -Force -Path (Join-Path $Workspace 'deps') | Out-Null @@ -116,6 +119,10 @@ BeforeAll { Push-Location $Workspace try { + if ($Capture) { + return @(& $resolver -DepsFile 'dependencies.txt' -Mode 'caller' -Seeds '' ` + -AdditionalSeeds '' -CloneRoot $CloneRoot 6>&1) + } & $resolver -DepsFile 'dependencies.txt' -Mode 'caller' -Seeds '' ` -AdditionalSeeds '' -CloneRoot $CloneRoot | Out-Null } @@ -323,6 +330,41 @@ Describe 'resolve-dependencies: ref selection across repeat invocations in one j (Get-Content $marker -Raw) | Should -Not -Match ([regex]::Escape($prefer)) } + # The guard for the residual risk in the fix: prefer_branch has one caller, and a second + # caller passing a ref that does not exist gets a silently correct-looking baseline built + # on the branch leg's dependency code. Warning only, deliberately: same-refs is the + # common and legitimate case whenever the pull request branch exists on no dependency. + It 'warns when a repeat resolution changed nothing' { + $ws = New-Workspace -Name 'guard-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Capture + + ($second -join "`n") | Should -Match 'Repeat resolution changed nothing' + ($second -join "`n") | Should -Match 'cancels out' + } + + It 'does not warn on a first invocation, which has nothing to compare against' { + $ws = New-Workspace -Name 'guard2-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard2-clones' + + $first = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Capture + + ($first -join "`n") | Should -Not -Match 'Repeat resolution changed nothing' + } + + It 'reports a notice, not a warning, when a repeat resolution did move' { + $ws = New-Workspace -Name 'guard3-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard3-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' -Capture + + ($second -join "`n") | Should -Match 'Repeat resolution moved 1 of 1' + ($second -join "`n") | Should -Not -Match 'Repeat resolution changed nothing' + } + It 'keeps the marker file out of the directories the caller junctions' { $ws = New-Workspace -Name 'rep3-ws' -Dependency $dep $root = Join-Path $sandbox 'rep3-clones' From 51eea99b5c35281c3212a462d0add9bdbe37fcac Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Mon, 24 Aug 2026 12:17:05 +0100 Subject: [PATCH 5/5] fix(resolve-dependencies): warn on a no-op repeat only when prefer_branch was explicit The guard warned whenever a repeat invocation left every ref unchanged, which is most baseline runs: the pull request branch usually exists on no dependency, so both passes legitimately resolve to the base branch. A guard that fires on the common case is worse than no guard. The script could not tell an explicit request from an inherited event default, because it saw only PR_BRANCH. The action now passes PREFER_BRANCH_EXPLICIT alongside it. Three outcomes: something moved emits a notice, an explicit request that moved nothing warns, an inherited default that moved nothing logs a plain line with no annotation. Still never an error. Failing on same-refs would red most pull requests. --- .../actions/resolve-dependencies/action.yml | 6 +++ .../scripts/Resolve-DependencyGraph.ps1 | 45 ++++++++++++----- ...ve-DependencyGraph.BaselineReuse.Tests.ps1 | 48 +++++++++++++------ 3 files changed, 71 insertions(+), 28 deletions(-) diff --git a/.github/actions/resolve-dependencies/action.yml b/.github/actions/resolve-dependencies/action.yml index 7cf75f8..f7366a1 100644 --- a/.github/actions/resolve-dependencies/action.yml +++ b/.github/actions/resolve-dependencies/action.yml @@ -149,6 +149,12 @@ runs: # script, which is why the baseline leg needed an input rather than a code change. PR_BRANCH: ${{ inputs.prefer_branch || github.event.pull_request.head.ref }} BASE_BRANCH: ${{ github.event.pull_request.base.ref }} + # Whether PR_BRANCH came from the input or from the event. The script cannot tell + # from the value alone, and the two mean different things on a repeat invocation: + # an explicit request that changed nothing is a misconfiguration, an inherited + # default that changed nothing is the ordinary case. See the no-op check at the end + # of Resolve-DependencyGraph.ps1. + PREFER_BRANCH_EXPLICIT: ${{ inputs.prefer_branch != '' }} DEP_TOKEN: ${{ inputs.token }} run: | $script = Join-Path $env:GITHUB_ACTION_PATH "scripts/Resolve-DependencyGraph.ps1" diff --git a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 index cd26a3f..b6623c0 100644 --- a/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 +++ b/.github/actions/resolve-dependencies/scripts/Resolve-DependencyGraph.ps1 @@ -383,25 +383,44 @@ if (Test-Path $selectFile) { # on the dependency side appears in both and cancels out when they are compared. Only reachable on # a repeat invocation, because a first pass has nothing to compare against. # -# WARNING, not an error, and it must stay that way. Same-refs is legitimate and common: it is -# what happens whenever the pull request's branch name exists on no dependency, which is most -# pull requests. Failing here would red every one of them. The point is that a reader of a -# baseline leg can see which case they are in without reconstructing it from two logs. +# The annotation level depends on WHO asked, which is why PREFER_BRANCH_EXPLICIT exists. +# +# explicit prefer_branch + nothing moved -> ::warning. A caller deliberately asked for a +# different ref and got the same one, so either the branch does not exist anywhere in the +# closure or the value is wrong. This is the misconfiguration worth interrupting for. +# something moved -> ::notice naming the moves. The normal baseline. +# inherited default + nothing moved -> plain log line, no annotation. This is the +# ordinary case: the pull request branch exists on no dependency, so both passes +# legitimately resolve to the base branch. An earlier version warned here and would have +# fired on most baseline runs, which is worse than not warning at all. +# +# Never an error. Failing on same-refs would red every pull request whose branch name exists on +# no dependency, which is most of them. $repeated = @($resolutionLog | Where-Object { $_.Previous }) if ($repeated.Count -gt 0) { - $moved = @($repeated | Where-Object { $_.Previous -ne $_.Selected }) - if ($moved.Count -eq 0) { - Write-Host ("::warning title=resolve-dependencies::Repeat resolution changed nothing: all " + - "$($repeated.Count) already-present dependenc(ies) stayed on the ref the previous " + - "invocation selected. Expected when the requested branch exists on no dependency. " + - "If this is a baseline pass that was meant to differ from its branch pass, the two " + - "are sharing dependency code and any regression in it cancels out. " + - "Check prefer_branch is set and names a branch that exists.") - } else { + $moved = @($repeated | Where-Object { $_.Previous -ne $_.Selected }) + $wasExplicit = $env:PREFER_BRANCH_EXPLICIT -eq 'true' + + if ($moved.Count -gt 0) { Write-Host ("::notice title=resolve-dependencies::Repeat resolution moved " + "$($moved.Count) of $($repeated.Count) already-present dependenc(ies): " + (($moved | ForEach-Object { "$($_.Key) $($_.Previous)->$($_.Selected)" }) -join ', ')) } + elseif ($wasExplicit) { + Write-Host ("::warning title=resolve-dependencies::prefer_branch was set explicitly to " + + "'$Prefer' but no dependency moved: all $($repeated.Count) already-present " + + "dependenc(ies) stayed on the ref the previous invocation selected. Either that " + + "branch exists on none of them, which is expected and harmless, or the value is " + + "wrong. If it is wrong, this pass is building against the same dependency code " + + "as the previous one, so any comparison between the two passes cannot see a " + + "regression that came from a dependency: it is present on both sides and " + + "cancels out.") + } + else { + Write-Host ("Repeat resolution changed nothing: all $($repeated.Count) already-present " + + "dependenc(ies) stayed on '$Prefer', which was inherited from the event rather " + + "than requested. Ordinary for a pull request whose branch exists on no dependency.") + } } } finally { diff --git a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 index 541bfbe..de74724 100644 --- a/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 +++ b/.github/scripts/tests/Resolve-DependencyGraph.BaselineReuse.Tests.ps1 @@ -107,15 +107,19 @@ BeforeAll { [string]$Fallback, # Return the script's host output instead of discarding it, for the tests that # assert on the workflow-command lines it emits. - [switch]$Capture + [switch]$Capture, + # Mirrors resolve-dependencies/action.yml's PREFER_BRANCH_EXPLICIT: whether + # PR_BRANCH came from the prefer_branch input or from the event payload. + [switch]$Explicit ) New-Item -ItemType Directory -Force -Path (Join-Path $Workspace 'deps') | Out-Null New-Item -ItemType File -Force -Path (Join-Path $Workspace 'deps/_shas.txt') | Out-Null - $env:PR_BRANCH = $Prefer - $env:BASE_BRANCH = $Fallback - $env:DEP_TOKEN = '' + $env:PR_BRANCH = $Prefer + $env:BASE_BRANCH = $Fallback + $env:DEP_TOKEN = '' + $env:PREFER_BRANCH_EXPLICIT = if ($Explicit) { 'true' } else { 'false' } Push-Location $Workspace try { @@ -330,28 +334,42 @@ Describe 'resolve-dependencies: ref selection across repeat invocations in one j (Get-Content $marker -Raw) | Should -Not -Match ([regex]::Escape($prefer)) } - # The guard for the residual risk in the fix: prefer_branch has one caller, and a second - # caller passing a ref that does not exist gets a silently correct-looking baseline built - # on the branch leg's dependency code. Warning only, deliberately: same-refs is the - # common and legitimate case whenever the pull request branch exists on no dependency. - It 'warns when a repeat resolution changed nothing' { + # The guard distinguishes who asked. An explicit prefer_branch that moved nothing is a + # misconfiguration worth annotating; the same outcome from an inherited event default is + # the ordinary case and must not annotate, because it happens on most baseline runs. + # Never an error either way. + It 'warns when prefer_branch was explicit and nothing moved' { $ws = New-Workspace -Name 'guard-ws' -Dependency $dep $root = Join-Path $sandbox 'guard-clones' Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' - $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Capture + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Explicit -Capture - ($second -join "`n") | Should -Match 'Repeat resolution changed nothing' + ($second -join "`n") | Should -Match '::warning title=resolve-dependencies::prefer_branch was set explicitly' ($second -join "`n") | Should -Match 'cancels out' } + It 'stays silent, with no annotation, when the default was inherited and nothing moved' { + $ws = New-Workspace -Name 'guard1b-ws' -Dependency $dep + $root = Join-Path $sandbox 'guard1b-clones' + + Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Capture + + # The case that made the first version of this guard useless: it fired here, which is + # most baseline runs. Informational only now. + ($second -join "`n") | Should -Not -Match '::warning' + ($second -join "`n") | Should -Match 'inherited from the event rather than requested' + } + It 'does not warn on a first invocation, which has nothing to compare against' { $ws = New-Workspace -Name 'guard2-ws' -Dependency $dep $root = Join-Path $sandbox 'guard2-clones' - $first = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Capture + $first = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' -Explicit -Capture - ($first -join "`n") | Should -Not -Match 'Repeat resolution changed nothing' + ($first -join "`n") | Should -Not -Match '::warning' + ($first -join "`n") | Should -Not -Match 'Repeat resolution' } It 'reports a notice, not a warning, when a repeat resolution did move' { @@ -359,10 +377,10 @@ Describe 'resolve-dependencies: ref selection across repeat invocations in one j $root = Join-Path $sandbox 'guard3-clones' Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer $prefer -Fallback 'develop' - $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' -Capture + $second = Invoke-Resolver -Workspace $ws -CloneRoot $root -Prefer 'develop' -Fallback 'develop' -Explicit -Capture ($second -join "`n") | Should -Match 'Repeat resolution moved 1 of 1' - ($second -join "`n") | Should -Not -Match 'Repeat resolution changed nothing' + ($second -join "`n") | Should -Not -Match '::warning' } It 'keeps the marker file out of the directories the caller junctions' {