Follow-on to #890 (merged), which added upstream-freshness verification to manual gr link --apply. The branch path works and is covered end to end. Two paths that #890 also added do not behave as its description states. Both were measured against the merged code by driving the gr binary, not by reading it.
Defect 1 — the detached-source refusals cannot fire for a branch-configured gripspace
ensure_gripspace_sources_current refuses a detached source in several cases; three of them were driven through the CLI — no recorded revision, a recorded revision that names a remote branch, and a recorded revision that does not resolve — and none of the three fires. All three apply the links and exit 0.
Mechanism. Manifest resolution re-materializes gripspaces before the freshness check runs. For a gripspace whose rev names a branch, checkout_rev finds refs/remotes/origin/<rev> and runs git checkout -B <rev> origin/<rev>, which re-attaches the detached HEAD and fast-forwards it. By the time the freshness check reads HEAD, the detached state it exists to judge has already been repaired, so head.is_branch() is true and control never reaches the detached branch at all.
Reproduction. Materialize a gripspace with rev: main, then:
git -C .gitgrip/spaces/<name> checkout --detach HEAD
git -C .gitgrip/spaces/<name> branch --show-current # empty: detached
gr link --apply # exit 0, links applied
git -C .gitgrip/spaces/<name> branch --show-current # main: silently re-attached
Discriminating control, same fixture and same gripspace, one variable changed, which is what rules out a bad fixture:
| HEAD state |
source behind upstream |
result |
| attached |
yes |
exit 2, Gripspace source '<name>' is behind origin/main by 1 commit(s) |
| detached |
yes |
exit 0, Applied 1 link(s) |
So the check runs, reaches the right gripspace, and works on the branch path. Detaching does not evade the comparison; the detached state is silently undone before the comparison happens.
Why the existing test does not catch it. detached_source_requires_recorded_pin_provenance calls ensure_gripspace_sources_current directly. The re-materialization sits between the CLI entry point and that function, so the test exercises the guard while skipping its caller. The guard is correct in isolation and unreachable in the product path, and a test anchored downstream of the step that erases its own premise passes regardless.
Defect 2 — a moved tag is compared against itself, so a stale source is certified current
A gripspace pinned to a tag materializes detached with the tag recorded, so the pin logic genuinely runs. When the tag moves upstream, the apply succeeds against stale content.
# rev: release, materialized at the tag
# upstream then commits and runs: git tag -f release
gr link --apply # exit 0
# upstream release = bc407a8, local release = 3c47afc, HEAD = 3c47afc
# composed output still carries the pre-move content
git fetch origin does not force-update an existing local tag, so revparse_single("release") resolves the stale local tag, which equals HEAD, and the pin matches. The branch path compares against refs/remotes/origin/<branch>, which fetch does update; the tag path compares against a ref fetch will not move. This is the same failure #890 exists to close — a successful local composition proving nothing about upstream currency — surviving inside the path #890 added.
An immutable full-SHA pin with upstream advanced is correctly accepted, which is right: a commit pin cannot go stale. That case passing is what localizes these two defects rather than implicating the whole check.
Consequence for the merged description
#890 states that it accepts a detached HEAD only when it matches an explicit tag or commit pin, and that it refuses rev-less or branch-configured detachment. Neither holds for branch-configured sources in the merged binary, and the tag half holds only when the tag has not moved.
Direction, offered rather than prescribed
- Run the freshness check before re-materialization, or have re-materialization not silently re-attach a deliberately detached source, so the detached refusals are reachable.
- Resolve a tag pin against the remote, or fetch tags with force, so a moved tag is compared to upstream rather than to itself.
- Move the detached-path coverage to the CLI boundary. A witness for a guard has to travel the path the product travels; this one currently starts after the step that removes the condition under test.
Follow-on to #890 (merged), which added upstream-freshness verification to manual
gr link --apply. The branch path works and is covered end to end. Two paths that #890 also added do not behave as its description states. Both were measured against the merged code by driving thegrbinary, not by reading it.Defect 1 — the detached-source refusals cannot fire for a branch-configured gripspace
ensure_gripspace_sources_currentrefuses a detached source in several cases; three of them were driven through the CLI — no recorded revision, a recorded revision that names a remote branch, and a recorded revision that does not resolve — and none of the three fires. All three apply the links and exit 0.Mechanism. Manifest resolution re-materializes gripspaces before the freshness check runs. For a gripspace whose
revnames a branch,checkout_revfindsrefs/remotes/origin/<rev>and runsgit checkout -B <rev> origin/<rev>, which re-attaches the detached HEAD and fast-forwards it. By the time the freshness check reads HEAD, the detached state it exists to judge has already been repaired, sohead.is_branch()is true and control never reaches the detached branch at all.Reproduction. Materialize a gripspace with
rev: main, then:Discriminating control, same fixture and same gripspace, one variable changed, which is what rules out a bad fixture:
Gripspace source '<name>' is behind origin/main by 1 commit(s)Applied 1 link(s)So the check runs, reaches the right gripspace, and works on the branch path. Detaching does not evade the comparison; the detached state is silently undone before the comparison happens.
Why the existing test does not catch it.
detached_source_requires_recorded_pin_provenancecallsensure_gripspace_sources_currentdirectly. The re-materialization sits between the CLI entry point and that function, so the test exercises the guard while skipping its caller. The guard is correct in isolation and unreachable in the product path, and a test anchored downstream of the step that erases its own premise passes regardless.Defect 2 — a moved tag is compared against itself, so a stale source is certified current
A gripspace pinned to a tag materializes detached with the tag recorded, so the pin logic genuinely runs. When the tag moves upstream, the apply succeeds against stale content.
git fetch origindoes not force-update an existing local tag, sorevparse_single("release")resolves the stale local tag, which equals HEAD, and the pin matches. The branch path compares againstrefs/remotes/origin/<branch>, which fetch does update; the tag path compares against a ref fetch will not move. This is the same failure #890 exists to close — a successful local composition proving nothing about upstream currency — surviving inside the path #890 added.An immutable full-SHA pin with upstream advanced is correctly accepted, which is right: a commit pin cannot go stale. That case passing is what localizes these two defects rather than implicating the whole check.
Consequence for the merged description
#890 states that it accepts a detached HEAD only when it matches an explicit tag or commit pin, and that it refuses rev-less or branch-configured detachment. Neither holds for branch-configured sources in the merged binary, and the tag half holds only when the tag has not moved.
Direction, offered rather than prescribed