Skip to content

fix(resolve-dependencies): resolve dependency refs on every invocation - #9

Merged
sakanni merged 5 commits into
developfrom
fix/serialisation-baseline-dependency-refs
Aug 24, 2026
Merged

fix(resolve-dependencies): resolve dependency refs on every invocation#9
sakanni merged 5 commits into
developfrom
fix/serialisation-baseline-dependency-refs

Conversation

@sakanni

@sakanni sakanni commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Problem

ci-serialisation runs the serialisation check on the PR branch. If that reports
errors, 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 root
at C:\bhom-deps. The second pass was not re-resolving anything. Two independent
causes:

  1. Clone-And-Checkout had 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.
  2. The branch the resolver prefers came from 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.txt is deleted at the start of every
invocation 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 HEAD cannot name the branch
it 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 is
    not.
  • resolve-dependencies/action.yml: new prefer_branch input, empty by default.
  • ci-serialisation/action.yml: the baseline pass passes prefer_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_branch currently has one
caller. 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_branch was set explicitly and nothing moved, and a
plain 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_branch behave identically. The input defaults to
empty and the action falls back to github.event.pull_request.head.ref, exactly as
before.

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-dependencies was
run 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, no
network 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

  • The cross-repository case. The end-to-end run used a closure whose dependencies
    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.
  • Cache-key divergence in a live run, for the same reason.
  • The guard's warning path in production. Covered by tests only.

Reviewers should expect the first real confirmation to be a ci-serialisation run
whose 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-serialisation when the baseline path runs.

This change does not alter which checks are required anywhere.

_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
sakanni force-pushed the fix/serialisation-baseline-dependency-refs branch from dd88618 to 51eea99 Compare August 24, 2026 13:53
@sakanni sakanni changed the title fix(resolve-dependencies): resolve dependency refs on every invocation (item 4) fix(resolve-dependencies): resolve dependency refs on every invocation Aug 24, 2026
@sakanni
sakanni merged commit 3a56da9 into develop Aug 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant