fix: preserve private mirror branches#30
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 7:22 AM ET / 11:22 UTC. Summary Reproducibility: yes. by source inspection: current main's Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land the mirror fix after removing the release-owned changelog entry and adding redacted real behavior proof for the branch-preservation and deleted-tracked-branch paths. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: current main's Is this the best way to solve the issue? Mostly yes: the mirror change is narrow, additive, and covered by temp-repo tests. The best mergeable version should drop the Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3f71740424ec. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 130b520ca7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if mode := info.Mode().Perm(); mode != 0o750 { | ||
| t.Fatalf("repo mode = %o, want 750", mode) |
There was a problem hiding this comment.
This new assertion is also exercised by the windows-test job in .github/workflows/ci.yml, which runs go test ./... on windows-latest. On Windows, Go's os.Chmod only maps to the read-only attribute and directory Mode().Perm() does not round-trip POSIX permissions like 0750, so this test will fail in CI even though the repository was created successfully; guard the POSIX permission check by platform or use a Windows-specific expectation.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
the
P2
ba7c157 to
88ac074
Compare
|
Landed in Proof: full tests, race tests, vet, dead-code analysis, and autoreview passed locally. GitHub Linux, Windows, CodeQL, secret scanning, race, and vulnerability gates passed on the merged head. |
|
Post-merge behavior proof for I ran an external Go harness against Focused regression proof: Full local proof: Exact merged-head GitHub proof:
The final changelog entry matches the verified behavior. No follow-up defect found; no corrective PR needed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88ac07442c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| trackedRemote, err := output(ctx, opts.RepoPath, opts.Git, "config", "--get", "branch."+opts.Branch+".remote") | ||
| if err == nil && strings.TrimSpace(trackedRemote) == "origin" { | ||
| return fmt.Errorf("tracked remote branch origin/%s is missing", opts.Branch) |
There was a problem hiding this comment.
For a local-only mirror branch created from another upstream, e.g. git checkout -b private origin/main, Git records branch.private.remote=origin but branch.private.merge=refs/heads/main. When origin/private does not exist, this check still reports the tracked branch as deleted even though the configured upstream is origin/main, so PullCurrent fails instead of preserving that unpublished local branch; also compare branch.<name>.merge to refs/heads/<name> before treating the missing ref as a deleted tracked branch.
Useful? React with 👍 / 👎.
| if opts.DirMode != 0 { | ||
| if err := os.Chmod(opts.RepoPath, opts.DirMode); err != nil { | ||
| return fmt.Errorf("set repo permissions: %w", err) |
There was a problem hiding this comment.
When DirMode is requested under an already-existing parent with broader permissions, git clone creates and populates RepoPath using Git's default directory mode before this post-clone Chmod narrows it. That leaves a window where other local users can read the cloned archive despite the caller asking for a private mirror; create/chmod an empty destination before cloning, or run the clone with a restrictive umask.
Useful? React with 👍 / 👎.
1 participant
Summary
Proof
go test ./...go test -race ./...GOWORK=off go vet ./...GOWORK=off go run golang.org/x/tools/cmd/deadcode@v0.45.0 -test ./...