Skip to content

Make the probe build-identity test say it cannot tell, rather than fail, when both probe crates share a version - #162

Merged
MikeGrier merged 2 commits into
mainfrom
mikegrier/probes-same-day-version-test
Oct 22, 2026
Merged

MikeGrier merged 2 commits into
mainfrom
mikegrier/probes-same-day-version-test

Conversation

@MikeGrier

Copy link
Copy Markdown
Owner

Summary

Fixes the five red CI jobs on the release PR (#160), which come from one test.

windows-platform-probes build_identity::tests::this_crate_reads_its_own_stamps_not_the_placement_probes asserted that this crate's version and windows-placement-probe's differ, because the version is the only thing that tells their build stamps apart. Both are calendar-versioned, so a release that bumps both on one day gives them one version, and #160 gives both 2026.1022.0. Any such release fails the same way.

The test now asks a pure function, whose_stamps, whether the versions can say whose stamps an identity carries: ours, the placement probe's, or not knowable when they coincide. In the last case it prints why and passes. The function is tested in both directions, including that equal versions answer neither way, and that an identity carrying neither version is an error rather than a reading.

What it leaves unchecked

On a day both crates are released, this_build() being rewired to the placement probe's BuildIdentity::current() is not detected. The compiler already refuses the other crate's stamp variables, so that rewiring is the one way left to read the wrong crate. The skip is visible only with the test's output shown.

This is option 1 of three put to the engineer. The structural fix, a crate name in BuildIdentity, would let the check hold on every date, but it changes windows-placement-probe's public type and what a report renders, so it was not chosen. It is recorded with that reason in the crate's design notes, and not scheduled. No sabotage entry covers the rewiring, since whether it is caught would follow the calendar; the manifest declares it in notCoveredHere.

Verification

  • With both crates temporarily given one version, the condition that failed CI: the old test failed, and the new one passes and prints that it cannot tell. The manifests were restored afterwards.
  • With today's differing versions the test takes the same path as before.
  • The crate's full suite with all features, clippy on all targets and features, the sabotage patch check and the encoding check pass.
  • A test commit triggers no release, so this PR does not change what chore: release main #160 proposes for any crate.

… probe crates apart says so instead of failing

this_crate_reads_its_own_stamps_not_the_placement_probes asserted that this crate's version
and windows-placement-probe's differ, to be sure it could tell their stamps apart. Both are
calendar-versioned, so a release of both on one day gives them one version, and the release
pull request that bumped both on 2026-10-22 failed five CI jobs on it.

The test now asks a pure function, whose_stamps, whether the versions can say whose stamps an
identity carries: ours, the placement probe's, or not knowable when the versions coincide. In
that last case it prints why and passes. whose_stamps is tested in both directions, including
that equal versions answer neither way. The case that failed CI was reproduced by giving both
crates one version, where the old test failed and the new one passes and prints the skip.

What this leaves unchecked on a day both crates are released is this_build() being rewired to
the placement probe's BuildIdentity::current(). That is recorded in the design notes with what
would change the answer (a crate name in BuildIdentity, not chosen), and declared in the
sabotage manifest's notCoveredHere, since an entry for it would be caught or missed by the
date.

Chosen by the engineer, 2026-10-22, from three options.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 22, 2026 19:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new classifier and its tests treat an unrelated identity as indeterminate when the expected versions match, contrary to the stated rejection behavior.

1 open issue
What changed in this PR

Adjusts the probe build-identity test to tolerate matching calendar versions, addressing the release-test failure described in #160 without changing production APIs.

Changes:

  • Adds a version-based classifier and tests for stamp attribution.
  • Reports when matching versions prevent distinguishing the two crates.
  • Documents the detection gap and why it has no sabotage entry.
File Description
crates/​windows-platform-probes/​src/​build_identity/​tests.rs Adds stamp classification and handles matching versions.
crates/​windows-platform-probes/​sabotage.json Records the version-dependent coverage limitation.
crates/​windows-platform-probes/​DESIGN-NOTES.md Documents the chosen approach and structural alternative.

Review effort: Balanced

Comment on lines +51 to +52
if ours == theirs {
Reading::CannotTell
Copilot AI balanced review requested due to automatic review settings October 22, 2026 19:40
@MikeGrier
MikeGrier enabled auto-merge October 22, 2026 19:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The classifier still accepts an unrelated identity as ambiguous when expected versions coincide, contrary to its stated error behavior.

1 open issue

Review effort: Balanced

@MikeGrier
MikeGrier merged commit 767b96c into main Oct 22, 2026
42 checks passed
@MikeGrier
MikeGrier deleted the mikegrier/probes-same-day-version-test branch October 22, 2026 19:53
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.

2 participants