fix(link): verify gripspace pins before applying

[laynepenney]

Summary

• Resolve included gripspaces from their existing checkouts during gr link --apply, so manifest loading cannot silently reattach a detached source before the freshness guard inspects it.
• Refuse an attached managed clone when its current branch differs from the configured revision, before comparing that branch with its own remote-tracking ref.
• Compare named tag pins with the tag advertised by origin, then make gr sync force-update managed tags so the recovery path reaches the same upstream object.
• Resolve hexadecimal commit-id abbreviations directly against the local object database after the origin-tag check, so a stale all-hex local ref cannot masquerade as an immutable commit pin.
• Refuse unresolved or ambiguous commit-id prefixes with remedies that can change the result.
• Exercise the behavior through the gr binary for a wrong attached branch, the matching attached-branch control, branch-configured detachment, missing and unresolvable provenance, full and abbreviated immutable commit pins, moved tags, and a deleted all-hex tag retained only by the managed clone.

Verification

• rustfmt --edition 2021 --check src/cli/commands/link.rs src/cli/dispatch.rs src/core/gripspace.rs tests/link_apply_detached_freshness.rs
• cargo test --test link_apply_detached_freshness -- --test-threads=1 (10 passed)
• cargo test (exit 0)
• Mutation: removing the configured-revision comparison makes the wrong-attached-branch witness accept and compose stale content while the matching-branch control remains green.
• Mutation: restoring preflight materialization makes the three detached-source witnesses fail while the controls remain green.
• Mutation: resolving the tag from the local clone makes the moved-tag refusal witness fail.
• Mutation: restoring a plain tag fetch preserves the refusal but makes the gr sync recovery witness fail.
• Mutation: resolving a commit-id prefix through ref-aware revision parsing makes only the deleted all-hex tag witness fail (1 failed, 9 passed).
• Mutation: requiring a full 40-character commit id makes only the abbreviated-pin witness fail (1 failed, 9 passed).
• Mutation: returning the bare object-database error removes the actionable remedy and makes only the operator-remedy witness fail (1 failed, 9 passed).

Ref #891 — closes at promotion.

[laynepenney]

r2 (Sentinel) — APPROVE, bound to head 47352e2.

RAN: exact-head checkout, cargo test --test link_apply_detached_freshness -- --test-threads=1 passed 7/7. I removed only the configured-revision comparison for an attached source. The wrong-branch witness then exited 0 and composed stale content. The source hash was restored exactly and all seven returned green.

READ: complete PR range and public platform text. The path distinguishes attached branch identity from detached recorded pins, resolves tags at origin, and keeps gr sync as the tested recovery path.

Scope: exact head, link-apply freshness semantics, and its end-to-end witnesses. Excludes later heads and merge mechanics.

[laynepenney]

r1 (Stromus): APPROVE — bound to head 47352e2b0cf5273b75e9bb7ce472be1f293c7dbe, base dev at 15242d0739a9e2948b68863ce0410044a1fbc6f8, 0 behind / 1 ahead.

RAN (range applied to a clean detached worktree at the base; the four touched files hash-checked after every step):

• rustfmt --edition 2021 --check clean on src/cli/commands/link.rs, src/cli/dispatch.rs, src/core/gripspace.rs, tests/link_apply_detached_freshness.rs
• cargo test --test link_apply_detached_freshness -- --test-threads=1: 7 passed
• full cargo test: exit 0, 56 result lines, 0 non-ok
• my own probe from the earlier round, re-run on the binary built from this head: a managed clone attached to scratch while the configured main advanced is now refused (exit 2, is on branch 'scratch', configured revision 'main'), the clone left on scratch, the composed file untouched
• mutation: removing only the configured-revision comparison turns exactly link_apply_refuses_an_attached_source_on_the_wrong_configured_branch red at the exit-code assertion with an empty diagnostic (apply exited 0, i.e. stale composition, not a setup failure); the six other tests including the matching-branch control stay green; restored and re-hashed 4/4

READ: the full patch, commit message and author/committer metadata, diff headers and paths, title, body, branch name. The configured-revision comparison runs right after branch identification and before the remote-tracking ref is built, so an attached clone on the wrong branch never reaches the behind check.

Scope: correctness and public sensitivity of this exact range, metadata, title, body, and branch name at this head; not a later head and not the merge. One non-blocking follow-on about gr sync in the same topology is tracked separately.

[laynepenney]

r1 (Apollo) — REQUEST CHANGES, bound to head 47352e2b0cf5273b75e9bb7ce472be1f293c7dbe, base dev, 0 behind / 1 ahead.

Both originally-reported defects are fixed, and I verified the recovery loop rather than just the refusal. One new false refusal blocks a documented configuration, with a recovery instruction that cannot resolve it.

Fixed — verified by running the original attacks

attack	result
recorded branch name at a detached commit	exit 2 — is detached from configured branch 'main'
provenance key missing	exit 2 — revision provenance is unavailable
recorded revision unresolvable	exit 2 — Cannot verify … against origin
full 40-hex SHA pin, upstream advanced	exit 0 — correctly still accepted

The ordering fix is the right shape: --apply now loads without materializing, so the detached premise survives to the guard instead of being repaired by checkout -B before it is observed.

The moved-tag path is fixed end to end, including the part that is easy to leave half-done. Upstream tag moved to a new commit → exit 2, composed output unchanged, no stale certification. Then the recovery the message names actually works: gr sync force-updates the local tag, the composed output advances to the upstream content, and the following apply exits 0. A refusal whose named recovery does not clear it would have been worse than the original defect, and this one clears.

Blocker — a short-SHA pin is refused permanently, and gr sync cannot fix it

is_full_commit_id requires exactly 40 hex characters. A revision that is neither a remote tag nor a full-length id now falls to an unconditional refusal. A short SHA is an ordinary pin: validate_rev accepts it, checkout_rev's own doc comment says "branch, tag, or SHA" without qualifying length, and materialization honours it correctly.

Discriminating pair — same fixture, same commit, both clones materialized to it, only the revision string's length differs:

rev=543e332347e8ffb330de6d403c42cdc6972a4bd2  len=40  clone HEAD correct=YES  apply exit=0
rev=543e3323                                  len=8   clone HEAD correct=YES  apply exit=2
   Error: Cannot verify gripspace source 'source-space' at configured revision
   '543e3323' against origin. Run `gr sync` before `gr link --apply`.

The clone is in exactly the state a correct short-SHA pin produces: detached, HEAD equal to the pinned commit, provenance recorded. Nothing is wrong with the workspace.

The recovery is a dead end. I ran gr sync and re-applied, twice: still exit 2, same message. gr sync cannot change the outcome, because nothing about the checkout is wrong — the refusal is about the length of a string in the manifest. An operator following the instruction loops.

Direction, offered rather than prescribed: resolving a short id against the local object database is sound for the same reason the 40-char case is — a commit id, however abbreviated, names an immutable object, so there is no staleness to guard against. Accepting any id that revparse_single resolves to a commit and confirming it is not also a remote tag would keep the moved-tag protection while restoring abbreviated pins. If abbreviated pins are meant to be unsupported, then the refusal should say so and name a real remedy, because "Run gr sync" is not one.

Scope of the regression claim

I am labelling this rather than asserting it: on dev the detached path called revparse_single(&rev) unconditionally, which resolves an abbreviated id, so this reads as introduced here. That is READ from the diff, not measured against a dev binary.

RAN

• Fresh clone at the exact head, 0 behind / 1 ahead, binary built from it; every case above built its own fixture from scratch.
• All four original attacks, the moved-tag case including its recovery, and the discriminating length pair.
• gr sync re-run after each refusal to test whether the named recovery clears it.

READ

Full range: all four changed files, the commit message, diff headers and paths. The non-materializing loader, the existing_gripspace identity resolution and its ambiguity refusal, the --force --tags fetch, and the branch-identity check on the attached path.

Scope

Correctness of this exact head against the two defects it repairs, plus the revision-resolution behaviour reachable from gr link --apply. Not the --status path, which retains the materializing loader. Not merge, and not a later head.

[laynepenney]

r2 (Sentinel) — REQUEST CHANGES, superseding my earlier approval at the unchanged head 47352e2.

READ: confirmed the short-SHA false-refusal seam. validate_rev and checkout_rev accept an abbreviated commit identifier, but link.rs accepts a detached commit pin only when is_full_commit_id requires exactly 40 hexadecimal characters. A correctly materialized detached short-SHA pin is therefore refused before composition, and the stated gr sync recovery cannot alter the short revision recorded by the manifest.

Apollo ran the discriminating full-SHA/short-SHA pair and the recovery twice. I did not repeat that fixture. The code path independently corroborates it.

Blocker: accept a uniquely resolving abbreviated commit pin while retaining remote-tag precedence and moved-tag refusal, or explicitly reject abbreviated pins with a remedy that can actually resolve the configured state. Add a short-SHA witness plus recovery control.

Scope: this correction is limited to the detached commit-pin preflight at this exact head. My earlier approval remains superseded.

[laynepenney]

r2 (Sentinel) — APPROVE, bound to head 92fff14.

RAN: applied the frozen v3 range to a fresh checkout at its declared base and ran the detached-freshness suite: 8 passed. Restoring the exact-40-character predicate produced one red, explicit_short_sha_pin_is_accepted, while the other seven remained green. Restored cleanly and re-ran 8 passed.

READ: the frozen range, metadata, title, and body. Scope: correctness and public-artifact sensitivity for the reviewed v3 head only. Remote tags retain precedence; abbreviated commit pins resolve to commits rather than failing only on their textual length.

[laynepenney]

r1 (Apollo) — REQUEST CHANGES on grip#892, bound to head 92fff14f54491aa5b515bbfbce305ad07838fc08.

My short-SHA blocker is genuinely fixed, and I confirmed it independently before looking for anything else. But the predicate that fixes it also opens a false accept in the same guard, and that is a worse failure direction than the one it repairs.

The short-SHA fix: confirmed — RAN

cargo test --test link_apply_detached_freshness at this head: 8 passed, 0 failed, including the new explicit_short_sha_pin_is_accepted. Re-tightening is_commit_id_like to rev.len() == 40 reds exactly one test — the short witness — with the other 7 green. The mutation claim holds as stated. Precedence is also as described, and stricter than described: remote-branch check, then origin-tag check, then commit-id-like, then refusal.

Blocker: an all-hex tag deleted from origin is now ACCEPTED from a stale local ref

remote_tag_commit asks origin whether the tag exists, which is what makes it a provenance check. When origin no longer has the tag it returns None — and the rev now falls through to revparse_single, which resolves local refs. A tag whose name happens to be all-hex therefore gets re-resolved from the managed clone's own stale copy, and the check passes.

Discriminating triple, one fixture, one variable changed per cell:

threshold	tag name	exit
>= 4 (this head)	cafe	0 — ACCEPTED
== 40 (before this commit)	cafe	2 — refused, remedy named
>= 4 (this head)	relx	2 — refused, remedy named

The third row is the control that matters: same commit, same fixture, same deleted-on-origin tag, and the only difference is whether the tag's name is hex. Opposite verdicts. That isolates the cause to the predicate rather than to the fixture.

The fixture is moved_tag_is_refused_until_sync_updates_the_managed_clone with release renamed to cafe and the upstream move replaced by git tag -d cafe. The clone's local tag survives the fetch (rev-parse cafe still resolves inside the space), and apply then composes against it and exits 0. This is the class that test exists to prevent, reached through a name.

It is worth being explicit about the direction: my earlier block was a false refusal — a correct configuration rejected, which is safe and merely obstructive. This is a false acceptance — provenance reported as verified against origin when origin was never consulted. The function's own docstring is the standard it fails: "A successful local composition proves that the files are internally usable. It does not prove that the source clone is current."

Fix direction, one line and it keeps the short-SHA behavior: after resolving, require that the resolved commit actually is what the rev abbreviates — commit.id().to_string().starts_with(&rev.to_lowercase()) — and fall through to the existing refusal when it does not. A genuine abbreviation satisfies this by definition; the stale cafe tag resolves to c05a5b7d…, which does not. That restores "this branch resolves object ids, not names," which is the property is_full_commit_id was silently providing before.

P1, separate and smaller: the new refusal names no remedy

Same-fixture pair with an unresolvable all-hex pin deadbeef:

• == 40: Cannot verify gripspace source '…' at configured revision 'deadbeef' against origin. Run ``gr sync`` before ``gr link --apply``.
• >= 4: Cannot verify gripspace source '…' at configured commit 'deadbeef': revspec 'deadbeef' not found; class=Reference (4); code=NotFound (-3)

The map_err on the new branch wraps every revparse failure into one shape with no next step, and that shape now covers three distinct operator situations: a typo, a commit that is genuinely gone, and — newly possible because abbreviations are accepted — an ambiguous prefix, whose actual remedy is "lengthen the pin" and is named nowhere. Appending the existing remedy sentence closes it.

Scope

RAN, against the exact head in a detached worktree: the full link_apply_detached_freshness suite; the == 40 re-tighten mutation; two probes of my own, each with its own discriminating control, restored and hash-verified after every run (link.rs back to e56f674a… each time). READ, not run: the rest of the test suite, and the ambiguous-prefix case — that one is a code-read of the shared map_err, not a measurement, and I am labeling it as such.

[laynepenney]

r2 (Sentinel) — APPROVE, bound to head 2ca8750.

RAN: exact-head checkout and cargo test --test link_apply_detached_freshness -- --test-threads=1: 10 passed. Replacing object-database prefix lookup with ref-aware revision parsing made only the deleted all-hex tag witness fail, accepting and composing stale local-tag content, while the other nine stayed green. Restored cleanly and re-ran 10 passed.

READ: the v4 delta and live public PR body. Scope: detached pin freshness and its focused binary witnesses at this head only. Origin tags retain precedence. Hexadecimal IDs then resolve only as commit-object prefixes, so stale local refs cannot certify an origin-absent tag.

[laynepenney]

r1 — APPROVE, bound to head 2ca875081c6e727572df5be61efed92ba364f147. Both of my exact-head findings from the previous revision are closed, and I verified the closure by mutation rather than by reading the new tests.

The fix is the right shape — RAN

commit_from_id_prefix resolves through the object database (odb().exists_prefix) instead of revparse_single. That is the property the guard needed: the ODB cannot see refs at all, so an all-hex name can no longer resolve, and ambiguous prefixes are rejected by the lookup itself rather than by a separate check. The refusal now also names two remedies where the previous revision emitted a bare libgit2 error with no next step.

Verification

• Focused suite at the exact head: 10 passed, 0 failed.
• Full cargo test: exit 0, 1141 passed across 56 test binaries, 0 failures. Captured by redirect rather than through a pipe, so the reported status is the compiler's and not a filter's.
• Mutation, mine, run independently: reverting the resolution to revparse_single reds exactly one test — an_all_hex_tag_deleted_from_origin_is_not_reinterpreted_as_a_commit_prefix — with 9 passing. link.rs restored and hash-verified afterwards.
• The discriminating detail that makes that mutation meaningful: explicit_short_sha_pin_is_accepted stays green under it. The two concerns are independently pinned, so the fix is not a blunt revert that happens to satisfy both.

A disclosure about my own instrument

I re-ran my previous revision's probe verbatim and it failed — exit 1 with Cannot identify one existing gripspace. That is a different error class from both the behaviour I was testing for (exit 0, the false accept) and the behaviour I expected (exit 2, a refusal), which means my fixture never reached the code path. It is an instrument failure, not a finding, and I am naming it rather than reporting it: the test file was substantially refactored in this revision and my probe was string-surgery against the previous fixture shape.

I therefore read the new witness instead of trusting its name. It constructs the same scenario — an all-hex tag present in the managed clone, deleted from origin, apply must refuse — and asserts two things mine did not: that the refusal leaves the clone unmoved, and that it does not recompose the output from the stale local tag.

Scope

RAN: the focused suite and the full suite at the exact head; one mutation with a restore verified by hash; my own probe (which failed as an instrument). READ: the diff against the previously blocked head, the new witnesses, and the ambiguous-prefix path — ambiguity is handled by exists_prefix and named in the refusal text, but constructing a real prefix collision needs a corpus this fixture does not build, so I did not exercise it.

[laynepenney]

r1 attribution (Apollo): comment 5354334950 above is my r1 APPROVE, bound to head 2ca875081c6e727572df5be61efed92ba364f147. The verdict body omitted the in-text display name; under a shared account that name is the attribution, so this addendum supplies it. Verdict and scope are unchanged.