Skip to content

fix(drift-audit): close four blind spots that hid the displayxr-common pin drift - #820

Merged
dfattal merged 1 commit into
mainfrom
fix/drift-audit-common-pin-blindspots
Aug 1, 2026
Merged

fix(drift-audit): close four blind spots that hid the displayxr-common pin drift#820
dfattal merged 1 commit into
mainfrom
fix/drift-audit-common-pin-blindspots

Conversation

@dfattal

@dfattal dfattal commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Why

The weekly drift-audit workflow ran green every Monday straight through the displayxr-common pin drift we cleaned up today — demos sitting on v2.0.0 / v2.1.0 / v2.3.1 while common was v2.5.0, and displayxr-cef-host pinned to a commit reachable from no branch in common (it resolved only because GitHub still serves unreachable objects; a GC would have broken that build with no warning).

The audit already had a displayxr-common pin spread check. It just couldn't see any of it. Four gaps:

# Gap Fix
1 PIN_CONSUMERS omitted every displayxr-demo-* repo added all five
2 CMAKE_CANDIDATES lacked common/CMakeLists.txt — where the demos declare their pins added; without it, fixing #1 alone still finds nothing
3 Non-semver pins (raw SHA/branch) fall out of the if latest and pk and lk guard silently now an explicit non-tag-pin finding
4 Submodule pins invisible to a FetchContent-only scan new .gitmodules pass

#3 is the one that mattered. 1/2/4 produce stale-but-valid pins — untidy. #3 hid a pin that could break a build with zero warning. The most dangerous case was the one the check was structurally unable to see.

Design notes

  • (4) is self-discovering on purpose. SUBMODULE_CONSUMERS is a bare repo list; paths come from .gitmodules. A hardcoded {repo: {path: dep}} map would rot the moment a repo restructures — and feat!: runtime owns the view math via XR_DXR_view_rig; drop the displayxr-common submodule displayxr-unreal#37 is dropping this submodule right now. Absent .gitmodules = silent skip, never a finding.
  • tag_for_sha() uses repos/:repo/tags for the dereferenced commit.sha. Release tags here are annotated, so git/ref/tags/:tag yields the tag-object sha, which never equals the commit a submodule pins — every lookup would miss and emit a bogus non-tag-pin. Cached per repo → one paginated call.

Verification

Run against the live org, the scan now discovers what it previously could not:

displayxr-demo-gaussiansplat       v2.5.0     <- previously (no common pin)
displayxr-demo-modelviewer         v2.5.0     <- previously (no common pin)
displayxr-demo-avatar              v2.5.0     <- previously (no common pin)
displayxr-demo-earthview           v2.5.0     <- previously (no common pin)
displayxr-unreal: Source/ThirdParty/displayxr-common
    sha=0e8001fd  resolved_tag=v2.5.0         <- previously invisible

unreal resolving to a tag (rather than None) is also the proof that the annotated-tag handling is right.

Synthetic checks cover the paths no repo exhibits anymore: raw-SHA is unrankable → takes the new branch; SHA pins extract from real CMake text; .gitmodules parses; absent .gitmodules yields no finding.

A full --dry-run is clean for common (everything is v2.5.0 as of today) and still reports the pre-existing unrelated displayxr-mcp v0.5.0→v0.5.2 lag.

Not in this PR

DRIFT_AUDIT_TOKEN is unset, so the workflow falls back to GITHUB_TOKEN: private repos are skipped and cross-repo issues can't be opened (the Jul 27 run only managed an issue on runtime itself). That's a secret to set, not a code change — flagging it so the audit's reach isn't overestimated.

🤖 Generated with Claude Code

https://claude.ai/code/session_011Y7bAdR1cFTNxadD5moCtk

…n drift

The weekly drift-audit ran green through the whole 2026-08 common-pin drift
(demos on v2.0.0/v2.1.0/v2.3.1 while common was v2.5.0, and cef-host pinned to
a commit reachable from no branch). Four gaps, all in the pin check:

1. PIN_CONSUMERS listed only runtime/shell-pvt/leia-plugin/cef-host — every
   displayxr-demo-* repo was never audited at all. Added all five.

2. CMAKE_CANDIDATES lacked common/CMakeLists.txt, which is exactly where the
   demos declare their FetchContent pins. Without it, adding the demos in (1)
   would still have found nothing.

3. A pin that is not a vX.Y.Z tag (raw SHA, branch) makes semver_key() return
   None, so the `if latest and pk and lk` guard skipped it in silence. That is
   precisely how cef-host's dangling-SHA pin stayed invisible — the most
   dangerous case was the one check couldn't see. Non-tag pins are now their
   own `non-tag-pin` finding.

4. Submodule-pinned consumers were invisible to a FetchContent-only scan
   (displayxr-unreal). Added a submodule pass that reads .gitmodules and
   resolves the pinned sha back to a tag.

(4) is deliberately self-discovering — SUBMODULE_CONSUMERS is a bare repo list
and paths come from .gitmodules — so it cannot rot when a repo restructures or
drops the submodule (unreal#37 is doing exactly that). A repo with no
.gitmodules is a silent skip, never a finding.

tag_for_sha() uses repos/:repo/tags for the DEREFERENCED commit.sha: release
tags here are annotated, so git/ref/tags/:tag returns the tag-object sha, which
never equals the commit a submodule pins — every lookup would have missed and
reported a bogus non-tag-pin. Cached per repo, so it costs one paginated call.

Verified against the live org: the scan now discovers common pins for
gauss/modelviewer/avatar/earthview (previously none) and resolves unreal's
submodule 0e8001fd -> v2.5.0. Synthetic checks cover the raw-SHA branch,
SHA extraction from CMake, .gitmodules parsing, and the absent-.gitmodules skip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Y7bAdR1cFTNxadD5moCtk
@dfattal
dfattal requested a review from a team as a code owner August 1, 2026 14:56
@dfattal
dfattal merged commit e1b07be into main Aug 1, 2026
21 checks passed
@dfattal
dfattal deleted the fix/drift-audit-common-pin-blindspots branch August 1, 2026 15:18
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