fix(resolve-dependencies): resolve dependency refs on every invocation - #9
Merged
Merged
Conversation
_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.
… 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.
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.
…hing 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.
…anch 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.
sakanni
force-pushed
the
fix/serialisation-baseline-dependency-refs
branch
from
August 24, 2026 13:53
dd88618 to
51eea99
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ci-serialisationruns the serialisation check on the PR branch. If that reportserrors, it checks out the base branch, rebuilds, runs again, and compares — so
failures that already exist on the base branch do not fail the PR.
Both passes call
resolve-dependencies, in the same job, against a shared clone rootat
C:\bhom-deps. The second pass was not re-resolving anything. Two independentcauses:
Clone-And-Checkouthad the clone and the ref resolution inside one conditional.A dependency already cloned by the first pass was left on whatever ref that pass
selected; only its SHA was re-recorded.
github.event.pull_request.head.ref,which is constant for the whole job, with no way for a caller to override it. So
the baseline pass had no means of asking for the base branch even in principle.
The result: the "baseline" was the base branch's own code built against the PR
branch's dependency code. Where a dependency also carried a branch of the same name
— which is how coordinated multi-repository changes work here — a regression
introduced in that dependency appeared identically in both passes, compared equal,
and the check passed.
Nothing in the output said so.
deps/_selection.txtis deleted at the start of everyinvocation and was written only when a clone was created, so the second pass produced
no dependency table at all and its log said nothing about which refs it was building.
What changed
Five commits, deliberately separable.
1. Report the resolved ref on every invocation. The selected ref is recorded
beside the clones and read back on reuse — a detached
HEADcannot name the branchit came from. Stored as a flat file, not a directory, because the calling action
junctions every directory under the clone root into the workspace parent and would
otherwise link it as if it were a dependency. There is a test for that. This commit
stands alone: it is the diagnostic that makes the defect visible, and it is useful
whether or not the rest lands.
2. Tests establishing the behaviour, before the fix.
3. The fix.
Resolve-DependencyGraph.ps1: the clone is now conditional, the ref resolution isnot.
resolve-dependencies/action.yml: newprefer_branchinput, empty by default.ci-serialisation/action.yml: the baseline pass passesprefer_branch: base_ref.Both halves are required. The tests show that an explicit base-branch request is
ignored while the clone is still present, so parameterising the branch alone does not
work.
This restores what the previous CI system did: it cloned each dependency if absent
and then always checked it out, with the branch as an explicit argument per pass.
4 and 5. A guard, added and then narrowed.
prefer_branchcurrently has onecaller. If a second passes a value
that resolves to the same refs, the baseline silently shares dependency code again.
On a repeat invocation the action now emits a notice naming any dependency that
moved, a warning when
prefer_branchwas set explicitly and nothing moved, and aplain log line when the value was inherited from the event and nothing moved. Never
an error: same-refs is the ordinary case whenever the PR branch exists on no
dependency, and failing there would red most pull requests.
Backward compatibility
Callers that do not set
prefer_branchbehave identically. The input defaults toempty and the action falls back to
github.event.pull_request.head.ref, exactly asbefore.
The resolution change is also inert for them, and this is the part worth checking:
before the fix, ref resolution already ran on fresh clones — the short-circuit
applied only to clones that already existed. A caller that resolves once has only
fresh clones, so it takes the same path as before and pays nothing. The added shallow
fetch is incurred only by a second invocation in the same job, which today is
ci-serialisation's baseline pass alone.Established two ways: the unchanged control tests assert that repeat resolution
against the same ref is stable, and every check that calls
resolve-dependencieswasrun cold against this branch. All resolved once, none emitted a re-resolution or a
guard line, and none regressed.
Verification
40 Pester tests, all passing — 15 in the new file, 25 pre-existing.
CI on this PR:
PowerShell script tests
The new tests are hermetic: a local bare repository reached via git
insteadOf, nonetwork and no BHoM install. They cover the resolver honouring the preferred branch
on a fresh clone, falling back when it does not exist, re-resolving an existing clone
when asked, moving the working tree rather than only the recorded SHA, producing
different cache-key inputs for the two passes, and all four guard outcomes.
Beyond the unit tests, the full two-pass path was exercised end to end against a
repository that exhibits a real serialisation failure, so the baseline pass actually
ran. It completed every step, the dependency table rendered for the second pass where
it had previously been empty, and the comparator reached the expected verdict.
What is not verified
all resolved to the same ref on both passes, so the two passes shared a cache key —
correct there, but it means the scenario the fix exists for, a dependency carrying a
branch of the same name as the PR, was not exercised. That needs a coordinated
multi-repository change, which is difficult to stage outside normal development.
Reviewers should expect the first real confirmation to be a
ci-serialisationrunwhose dependency table shows the two passes on different refs. Commit 1 exists so
that is visible rather than inferred.
Cost
One extra shallow fetch per dependency per additional invocation, and the baseline
pass now misses the branch pass's assembly cache and builds its own closure. That is
the intended trade — the two passes are supposed to differ — but it is a real
increase for
ci-serialisationwhen the baseline path runs.This change does not alter which checks are required anywhere.