Repository navigation
fix(refs): keep missing references from resolving to lookalike tags - #2316
Merged
Merged
Conversation
<!-- agent --> Fetching a valid tag named `refs/heads/unborn` stores it under `refs/tags/refs/heads/unborn`. When `HEAD` names the absent `refs/heads/unborn`, the CLI reference reader used `git rev-parse --verify` and guessed that tag instead. `head.is_valid()` became true and `IndexFile.commit()` gave the local initial commit a foreign parent. A tag named `REBASE_HEAD` also made `Repo.currently_rebasing_on()` report a rebase. Read direct references with `git show-ref --verify --hash` instead of revision parsing. Probe `REBASE_HEAD` with `git show-ref --exists` to distinguish absence from lookup errors, then use `SymbolicReference.dereference_recursive()` and `Repo.commit()` so the rebase reader shares exact lookup and symbolic-reference validation while retaining recursive annotated-tag peeling. Revision APIs intentionally retain Git's name guessing. Existing native Gix reference reads are unchanged; root-reference existence and reftable retain documented CLI fallbacks. Independently written regressions fetch the misleading tags into an unborn repository and cover SHA-1/SHA-256, files/reftable, forced CLI fallback, parentless initial commits, and absent/present/deleted `REBASE_HEAD`. Positive cases compare existing root refs, packed annotated tags, and detached `HEAD` against Git. All 16 original regressions failed on the CLI baseline; 14 failed under Gix, whose files-based initial commits were already protected. All 26 final reference regression cases pass on each backend. Post-commit review found that `SymbolicReference.commit` peels only one annotated tag; four new regressions failed with `TypeError` before switching to the existing recursive commit resolver. Broader validation exposed the existing GIX-17 grammar bug for this work branch: Gix reads `2313-supersede` as blob `23136a1d1dd1f02de7cde0f7b55f49d76ac77d55`, while Git reads the branch's commit. Extend the existing conservative CLI grammar guard to hexadecimal prefixes followed by a dash. A deterministic fixture verifies existing `<blob-prefix>-topic` and missing `<blob-prefix>-missing` names; its test fails before the guard and passes afterward. The GIX-17 ledger retains the upstream bug as open and records the evidence and adapter fallback. An additional exact-name check exposed GIX-33: when the root `REBASE_HEAD` is absent and `refs/tags/REBASE_HEAD` is symbolic, Gix's partial lookup returns that tag and native `symbolic_ref` dispatch follows its target. Git's equivalent `symbolic-ref --quiet --no-recurse` exits 1. Compare the returned native reference name with the requested bytes and retain CLI for a lookalike. Two new SHA-1/SHA-256 regressions fail before the guard and pass afterward, including forced CLI fallback. The ledger keeps this compatibility bug open until Gix exposes a verified exact-name mode. Final caller validation passes 98 CLI reference tests and 200 Gix reference/backend tests, with 24 passing subtests each. The preceding intermediate commit `9ddf778` also passed the full local suites: 1,969 CLI tests and 2,072 Gix tests, with 85 passing subtests each. Earlier targeted revision/installation checks passed 48 CLI tests and 50 Gix tests. Ruff check/format, mypy, basedpyright, ShellCheck, and `git diff --check` pass. Behavioral comparisons use `git version 2.54.0 (Apple Git-157)` and `GixPython 0.3.0`. The Git reference checkout at `6de20f6092dcf9bdb1c8efe03db4b70c82b423dd` documents exact lookup in `Documentation/git-show-ref.adoc`, name guessing in `Documentation/revisions.adoc`, and parentless root commits in `Documentation/git-commit-tree.adoc`. Source report and investigation: Keerthana-64's PR #2313. The replacement does not incorporate that PR's implementation or tests. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol <codex@openai.com>
The existing `AGENTS.md` guidance requires PRs to describe the problem, but only commit bodies explicitly require motivation. Port the PR motivation requirement from the Gitoxide worktree so reviewers can understand why an agent pursued the work. Require agent-created PR descriptions to identify the underlying motivation, such as the benefiting project or where the bug was discovered. Clarify that restating the fix is insufficient. Validation: reviewed the wording against the Gitoxide requirement and ran `git diff --check`. No tests were run because this changes repository instructions only.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The credential-redaction test misses normalized Windows source paths, causing false assertion failures.
1 open issue
What changed in this PR
GitHub Copilot: This PR makes reference reads exact, preventing lookalike tags from supplying missing branches or falsely signaling a rebase, while preserving revision-name guessing.
Changes:
- Use exact CLI reference lookups and guard Gix symbolic-reference results.
- Add regression coverage for lookalike tags, initial commits, nested tags, and hexadecimal-prefixed revision names.
- Document compatibility fallbacks and update contributor guidance and credential-redaction tests.
| File | Description |
|---|---|
| tests/fixtures/rev-parse.sh | Add a hexadecimal-prefix collision fixture. |
| tests/fixtures/exact-refs.sh | Create lookalike-tag and exact-reference scenarios. |
| test/test_rev_parse.py | Test hexadecimal-prefixed branch and missing names. |
| test/test_refs.py | Cover exact lookups, initial commits, and rebase detection. |
| test/test_git.py | Exclude fixture paths from credential assertions. |
| git/repo/base.py | Detect rebase state through the exact reference. |
| git/refs/symbolic.py | Replace revision guessing with exact reference verification. |
| git/_backend.py | Guard Gix name matching and revision grammar. |
| doc/source/changes.rst | Describe exact-reference behavior and revise the heading. |
| doc/gix-backend.md | Document CLI fallbacks and open Gix compatibility bugs. |
| AGENTS.md | Require underlying motivation in agent-created PR descriptions. |
Review effort: Balanced
The Ubuntu 3.14t CI job rejected correctly redacted clone output because its randomly generated `/tmp/.tmpg8ibob` fixture path contained `bob`. The credential assertions scanned the whole error and trace, so ordinary source and destination paths could masquerade as leaked credentials. Replace only the known fixture directory with `<fixture>` before checking the error and trace for credentials. Keep the exact redacted-option assertions and the credential checks outside that directory. Include `alice-bob-user-token` in the input paths so this false positive is covered deterministically, rather than depending on a random name. The Windows CLI and Gix jobs then failed all four trace subtests because `Repo._clone()` uses `Git.polish_url()` for the source path: the command and stderr contain forward slashes, while `str(directory)` contains backslashes. Copilot's review requested: "Remove both the native and POSIX spellings of the fixture directory before checking for credentials." Remove both exact spellings from the error and trace to address that feedback without weakening the credential checks elsewhere. Focused CLI and Gix runs each pass three redaction tests and four subtests. An assert-based check runs the new replacement loop against four actual Windows CLI/Gix CI failure messages: all pass, and a credential outside the fixture path remains visible. `ruff check`, `ruff format --check`, and `git diff --check` also pass. Windows execution remains for remote CI.
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.


Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by
Codex GPT-6.Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Supersedes: #2313. This is an independent solution to the underlying problem reported and investigated by @Keerthana-64.
Motivation: This work was pursued in response to @Keerthana-64's investigation in #2313. Independently reproducing the report provides evidence for addressing it across GitPython's CLI and Gix backends, including compatibility gaps uncovered during validation.
Problem: Fetching a tag named
refs/heads/unbornstores it asrefs/tags/refs/heads/unborn. IfHEADpoints to the missing branchrefs/heads/unborn, GitPython's CLI reference reader guesses the tag:head.is_valid()returns true, andrepo.index.commit("initial")incorrectly gives the first local commit a parent from the remote's history. A tag namedREBASE_HEADsimilarly makescurrently_rebasing_on()report a rebase when none is in progress. Missing references must stay missing, initial commits must have no parent, and rebase detection must read the actual rebase reference.Relationship to Git: Git distinguishes exact reference names from revision shorthand.
git show-ref --verifyrequires an exact path;git rev-parseapplies the name-guessing rules documented ingitrevisions. The regression fixture confirms that Git rejects the unbornHEADand reports the exact references absent despite the fetched tags. This restores Git-compatible reference reads; revision APIs retain their intentional name guessing.Solution: Use
show-ref --verify --hashin the shared direct-reference reader. ProbeREBASE_HEADwithshow-ref --exists, then dereference its exact name throughSymbolicReferenceand useRepo.committo preserve recursive annotated-tag peeling. Native Gixsymbolic_refdispatch also verifies the returned reference name, preventing symbolic lookalike tags from impersonating missing root refs (GIX-33). Validation exposed GIX-17 for the prepared branch name2313-supersede: Gix interpreted the hexadecimal prefix as an object ID. Extend the existing CLI grammar fallback to that form, preserving Git's branch/missing-name behavior. Both upstream compatibility bugs remain open. Root-reference probes and reftable retain documented CLI fallbacks; there is no new native Gix coverage claim.Validation: All 16 original regression cases fail before the fix on CLI; 14 fail under Gix, which already protects files-based initial commits. All 26 final reference cases pass on both backends, covering SHA-1/SHA-256 and files/reftable. The final symbolic-name guard also has two demonstrated pre-fix Gix failures. Latest caller suites pass 98 CLI reference tests and 200 Gix reference/backend tests, plus 24 passing subtests each. Before that final guard, full local suites passed 1,969 CLI tests and 2,072 Gix tests. Final post-commit Codex review found no actionable issues. Ruff check/format, mypy, basedpyright, ShellCheck, and
git diff --checkpass. CI passes all 67 checks ond400a1d, including the full Windows CLI and Gix matrix. A test-only follow-up excludes both native and POSIX spellings of fixture paths before credential assertions, addressing the Windows failures and review feedback. Focused redaction checks pass three tests and four subtests on each backend; the actionable review thread is resolved.Reproduction, Git evidence, and validation details
The independently written
tests/fixtures/exact-refs.shcreates a remote commit, tags it with names resembling missing branch, tag, remote, and root references, and fetches those tags into a repository whoseHEADnames an unborn branch. No source-PR code or tests were applied or copied.The regression checks:
git rev-parse --verify --quiet HEADexits 1.git show-ref --exists -- <exact-name>exits 2 even thoughrefs/tags/<exact-name>resolves successfully.ValueError, andhead.is_valid()returns false.refs/tags/REBASE_HEADcannot supply a missing rootREBASE_HEAD. Gix's partial lookup returned the tag's name and target while Git's exactsymbolic-ref --quiet --no-recurseexited 1; two SHA-1/SHA-256 cases failed before the native name guard. Both native dispatch and forced CLI fallback now raiseValueError.IndexFile.commit()creates a parentless commit without changing the fetched tag.REBASE_HEADtag does not signal a rebase; creating and deleting the actual reference changes detection correctly.REBASE_HEADpointing to nested annotated tags still resolves recursively to its commit. Four review regressions failed withTypeErrorbefore the correction.HEAD,FETCH_HEAD,ORIG_HEAD,MERGE_HEAD,REBASE_HEAD,refs/stash, and packed annotated tags match Git's object IDs. DetachedHEADstill resolves.Repo.commit("REBASE_HEAD")retains revision-name guessing. Reference lookup is also checked with forced CLI fallback.The additional GIX-17 regression creates
<blob-prefix>-topicpointing to a different commit. GixPython 0.3.0 returned the blob for that branch name and for nonexistent<blob-prefix>-missing; Git returned the branch's commit and rejected the missing name. Both are now handled through the existing CLI grammar fallback. This is an adjacent compatibility correction found during validation, beyond the source PR's original exact-reference fix; the upstream ledger entry stays open.Behavioral comparisons use
git version 2.54.0 (Apple Git-157), CPython 3.12.14, and GixPython 0.3.0. The separately inspected Git reference checkout is at6de20f6092dcf9bdb1c8efe03db4b70c82b423dd:Documentation/git-show-ref.adoc: exact-path verification and existence exit codes.Documentation/revisions.adoc: reference-name guessing order.Documentation/git-commit-tree.adoc: initial/root commits have no parents.Final caller checks on
fa4e329:Earlier targeted revision/installation checks:
Full local checks on intermediate commit
9ddf778, before the final symbolic-name guard:Gix's report records native
Reference.readfor supported exact lookups,show_ref: CLI: not convertedfor the new fallback calls, and the existing GIX-1 reftable fallback.doc/gix-backend.mdrecords the remaining root-reference capability/verification gap.