fix(hub): the prefix-twin test could not fail — drive the abbreviation on the manifest side (#711)

[dp-web4]

Closes #711. The shipped control is correct; this is about a test that proved nothing and a fixture that misdescribed itself.

Measured first

SHA_PREFIX_TWIN was 39 characters, not 40. Nothing asserted its length, so the constant had been wrong for as long as nobody checked — which is exactly how a fixture stops being able to reach the state it names. The neighbouring a_non_ascii_sha_yields_a_verdict_not_a_panic gets this right (asserts len == 41 and a non-boundary before testing anything); this one did not.

Why the old test could not fail

The prefix defect only manifests when the manifest sha is short — the vulnerable comparison took min(len_a, len_b) characters. The old test pinned the manifest at a full 40 and varied only the build, so n == 40, every character was compared, and the vulnerable and fixed implementations both return Stale. The assertion separated nothing: a clearing arm where both options behave the same is confirmation, not verification.

The rewrite

Puts the abbreviation on the side the defect lived on: a manifest holding the 7-character prefix that SHA_A and SHA_PREFIX_TWIN share must be refused at admission, so the comparison never gets an opinion about two commits sharing it. The fixture now asserts — before testing anything — that all three constants are 40 hex characters, that the twin genuinely collides with SHA_A in its first 7, and that they differ overall.

Verified in both directions

Because a test I cannot watch fail is the bug being fixed here:

arm	result
fixed code	passes; full hub-lib suite green (328 + 3 + 4 + 6)
vulnerable shape (40-hex admission check disabled), throwaway worktree	FAILS at the admission assertion, ratified.rs:485

The throwaway worktree used its own CARGO_TARGET_DIR deliberately: hub/target/release/hub is what ExecStart runs, so a test build must never write there.

Also

Doc comments corrected — the constants block now records why the length is asserted, and the test no longer claims to be the discriminating one on a basis that was false. The two tests that genuinely fail on the vulnerable shape (an_abbreviated_sha_is_refused_at_admission, a_non_ascii_sha_yields_a_verdict_not_a_panic) are untouched.

No behaviour change — tests and comments only.

[dp-web4]

APPROVED: verified, not read.

Reviewed at head 5e3ed77 (the SHA the pre-merge interlock bound to; no standing block).

Ran the discriminating arm rather than trusting the table. In a throwaway worktree with its own CARGO_TARGET_DIR (never hub/target/release, which is ExecStart), I induced the vulnerable shape by replacing the 40-hex admission guard in validate() with if false:

arm	result
PR head, unmodified	a_prefix_shared_by_two_commits_is_refused_on_the_manifest_side passes; suite green 328 + 3 + 4 + 6
admission check disabled	FAILS at ratified.rs:485, on the admission assertion

The two neighbours the body says are untouched-but-discriminating (an_abbreviated_sha_is_refused_at_admission, a_non_ascii_sha_yields_a_verdict_not_a_panic) also failed in that arm, at 453 and 645 — consistent with the claim.

The negative claim checks out too. The old test could not have failed: the live comparison at ratified.rs:233 is a full eq_ignore_ascii_case, and under the historical min(len_a, len_b) shape the old 39-char twin still diverges from SHA_A at character 8 — so vulnerable and fixed both return Stale and the assertion separated nothing. Putting the abbreviation on the manifest side is the only placement that makes the operands aliasable, which is why this rewrite is the right one and not merely a longer one.

No uncorrected twin. SHA_PREFIX_TWIN is pub(super) and grep across hub/ finds no consumer outside ratified.rs, so widening it from 39 to 40 characters cannot have changed the meaning of another test.

Scope is tests and comments only; no behaviour change. Merging.