Make the ambiguous-prefix test actually test ambiguity - #651
Merged
Conversation
CommitLore — record lintTrailers: clean — 1 commit in Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
MongLong0214
force-pushed
the
fix-649-ambiguous-prefix
branch
from
August 14, 2026 07:02
9f74258 to
3366ac5
Compare
The test asked git to resolve a three-hex name and asserted the refusal. Git
refuses any name shorter than four hex for its length alone, so the assertion
was answered before ambiguity was ever consulted, and the 120-commit search
that fed it changed nothing. It also returned early — reported as a pass — in
the 17% of runs that found no collision, so neither branch of the test proved
anything.
Four hex now, with the colliding object forged rather than hunted. It has to
be a commit: for a `^{commit}` query git disambiguates by type, so a blob
sharing the prefix is passed over and the name still resolves. Searching in
process costs one write instead of hundreds of commits, and removes the
growing index that CI was building at the moment a staged object went missing.
Blast: local
Undo: easy
Certainty: firm
Ruled-out: keep committing until two ids collide | 4-hex needs ~300 commits by the birthday bound, and the loop was already the heaviest thing in the file
Ruled-out: forge a colliding blob instead of a commit | measured: `rev-parse <prefix>^{commit}` disambiguates by type and resolves anyway
Limit: this does not explain the object-database failure in #649; the loop was removed for its own defects, not as a fix for that
Provenance: authored
Record-Id: r-ambigprefix649
Verified: negative control — removing the forged commit fails the test with "expected +0 not to be +0"; restored, 189 cases across sha256-repo, git and query; tsc --noEmit clean
MongLong0214
force-pushed
the
fix-649-ambiguous-prefix
branch
from
August 14, 2026 07:26
3366ac5 to
2329e37
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.
Addresses the test defect described in #649. Not a close: the unexplained object-database failure recorded in the same issue is untouched here, and the issue must stay open for it.
What was wrong
test/sha256-repo.test.ts>refuses an ambiguous prefix rather than picking oneasked git to resolve a three-hex name:Git's minimum abbreviation is four hex, so a three-hex name is refused for its length:
The assertion was therefore answered before ambiguity was ever consulted. The 120-commit collision search that fed it changed nothing about the outcome. The test named for ambiguity has never exercised ambiguity.
That is worse than the silent-pass defect filed in #649, which said the test proved nothing in ~17% of runs. It proved nothing in all of them.
Two measurements that shaped the fix
Forging a colliding blob does not work — git disambiguates by type for a
^{commit}query:Forging a commit does:
The fix
Four hex, and the second commit is forged by searching the
commit <len>\0…preimage in process — 39k iterations in the measured case, no spawns, and only the winning body is written. The collision exists by construction, so there is no early return and no path where the test reports green without asserting.Negative control
Removing the forged commit fails the test:
Restored: 189 cases pass across
sha256-repo,gitandquery;tsc --noEmitclean.What this does not claim
The 120-commit loop was the heaviest thing in the file and it is gone, which removes the growing index that CI was building when a staged object went missing on run 31771047613. That is a side effect, not a diagnosis. #649 still holds the unexplained failure, and this PR should not be read as closing it.