Skip to content

Make the ambiguous-prefix test actually test ambiguity - #651

Merged
MongLong0214 merged 1 commit into
mainfrom
fix-649-ambiguous-prefix
Aug 14, 2026
Merged

Make the ambiguous-prefix test actually test ambiguity#651
MongLong0214 merged 1 commit into
mainfrom
fix-649-ambiguous-prefix

Conversation

@MongLong0214

@MongLong0214 MongLong0214 commented Aug 14, 2026

Copy link
Copy Markdown
Owner

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 one asked git to resolve a three-hex name:

const prefix = sha.slice(0, 3);
const direct = execGit(['rev-parse', '--verify', '--quiet', `${ambiguous}^{commit}`], { cwd });
expect(direct.code).not.toBe(0);

Git's minimum abbreviation is four hex, so a three-hex name is refused for its length:

$ git rev-parse --verify "${SHA:0:3}^{commit}"
fatal: Needed a single revision

3-hex => refused      4-hex => resolves      5-hex => resolves

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:

commit 0cb55f99…  forged blob 0cb5ace1…  (share "0cb5")
$ git rev-parse --verify --quiet 0cb5^{commit}
0cb55f9926cbf87fc05c7db6b0062d4d2bf3d15d      <- resolves, collision ignored

Forging a commit does:

$ git rev-parse --verify 0cb5^{commit}
error: short object ID 0cb5 is ambiguous
hint: The candidates are:
hint:   0cb55f9 commit 2026-08-14 - seed

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:

AssertionError: expected +0 not to be +0
Tests  1 failed | 24 skipped (25)

Restored: 189 cases pass across sha256-repo, git and query; tsc --noEmit clean.

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.

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

CommitLore — record lint

Trailers: clean — 1 commit in origin/main..2329e37977327685fe145026ab1512cc9f4a8167
Active constraints: not read — commitlore: the index is incomplete: the build stopped after its time budget with 666 commit(s) or note(s) unread — records in them are missing from this answer. fix: commitlore init (or commitlore index) to finish the index (1 changed path)

Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR.

@MongLong0214
MongLong0214 force-pushed the fix-649-ambiguous-prefix branch from 9f74258 to 3366ac5 Compare August 14, 2026 07:02
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
MongLong0214 force-pushed the fix-649-ambiguous-prefix branch from 3366ac5 to 2329e37 Compare August 14, 2026 07:26
@MongLong0214
MongLong0214 merged commit 7c57e04 into main Aug 14, 2026
12 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