Repository navigation
Add Git attribution and profile email selection - #14924
Draft
williammartin wants to merge 3 commits into
Draft
williammartin wants to merge 3 commits into
williammartin wants to merge 3 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 965836ef-19ac-4473-ba1f-e326ce0f106b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 965836ef-19ac-4473-ba1f-e326ce0f106b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 965836ef-19ac-4473-ba1f-e326ce0f106b
2 of 6 tasks
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Multiple matching owner identities can silently apply the wrong Git name/email through include ordering, and regression tests are missing for attribution clearing and save-time overlap validation.
3 open issues
What changed in this PR
This draft prototype extends the identity-selection stack with optional Git commit attribution, separate from authentication. It builds on owner-based selection and is explicitly not intended to merge as-is.
Changes:
- Store optional Git names and emails in identity files, with edit and clear support.
- Resolve
@publicand@noreplyselectors once and save concrete addresses. - Extend attribution tests across scopes and worktrees, and add an isolated macOS replay.
| File | Description |
|---|---|
| script/multiaccount-review/isolated-runner.sh | Sandboxes offline replay against network and keychain access. |
| script/multiaccount-review/identity-scenario.sh | Adds isolated attribution and SSH-routing scenarios. |
| script/multiaccount-review/05-attribution.sh | Provides the attribution replay entrypoint. |
| pkg/cmd/root/account_git_test.go | Uses identity-provided attribution in account-routing tests. |
| pkg/cmd/auth/use/worktrees_test.go | Checks attribution inheritance and precedence across worktrees. |
| pkg/cmd/auth/use/use_test.go | Checks attribution in future descendant repositories and reorganizes tests. |
| pkg/cmd/auth/identity/identity.go | Adds attribution flags, clearing, and selector resolution. |
| pkg/cmd/auth/identity/identity_test.go | Tests attribution round trips and legacy metadata preservation. |
| pkg/cmd/auth/identity/email.go | Resolves public and noreply profile addresses. |
| pkg/cmd/auth/identity/email_test.go | Tests account selection, fixed snapshots, and lookup failures. |
| internal/identity/store.go | Persists attribution defaults and validates overlapping scopes. |
| internal/identity/store_test.go | Tests live attribution updates, precedence, and account-only replacement. |
| internal/identity/hardening_test.go | Adds Unicode, worktree inheritance, and cross-host overlap coverage. |
Review effort: Balanced
Comment on lines
+538
to
+539
| if a.Owner != "" && b.Owner != "" { | ||
| return false |
Comment on lines
+106
to
+107
| if err := s.validateDirectoryAttribution(bindings, &id); err != nil { | ||
| return err |
Comment on lines
+97
to
+101
| if clearGit { | ||
| if cmd.Flags().Changed("git-user-name") || cmd.Flags().Changed("git-user-email") { | ||
| return cmdutil.FlagErrorf("--clear-git cannot be combined with --git-user-name or --git-user-email") | ||
| } | ||
| id.GitUserName, id.GitUserEmail = "", "" |
This branch has not been deployed
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.


Description
Follow on from #14923
Choosing the authentication account does not choose the name and email used on Git commits. This PR adds optional Git attribution to identities and explicit
@publicand@noreplyemail lookup.Layer 5 of an eight-part prototype review stack. This is a way to review the design and implementation in smaller pieces, not a proposal to merge the current stack as-is.
How did you test this change?
I inspected the identity file containing Git attribution, made local commits in the main checkout and a linked worktree, and checked shared edits. I also exercised real profile lookups, fixed-address snapshots and a failed public-email lookup that preserved the previous value.
A real noreply profile lookup is a fixed snapshot
multiaccount-layer-5-02-A-real-noreply-profile-lookup-is-a-fixed-snapshot.mp4
Literal attribution follows the repository family
multiaccount-layer-5-03-Literal-attribution-follows-the-repository-family.mp4
Missing public email does not overwrite an identity
multiaccount-layer-5-04-Missing-public-email-does-not-overwrite-an-identity.mp4
Key points
The included identity file supplies ordinary
user.nameanduser.emaildefaults. Authentication remains separate, and Git can still apply stronger local or environment settings. These defaults do not independently distinguish author and committer.Profile selectors resolve once and store a concrete address. Changing unrelated identity settings does not silently refresh it. The literal email addresses in local commit demonstrations are examples, not claims about an account profile.
Notes for reviewers
The profile-snapshot recording changes the identity account to demonstrate the current snapshot behaviour. I am not sure account edits should be supported at all. That unresolved design question is called out in the identity foundation PR, not endorsed by this recording.
The recordings use the product code at this layer, not a later stack endpoint. The included offline replay scripts are supplementary and do not reproduce the live-account recordings.
Offline replay on macOS
Build this layer, then run its synthetic scenario with an absolute executable path. This is a replay instruction, not a claim that the command below was used to make the videos.
Authorship and follow-up
Who wrote this:
Who answers review comments:
Acceptance Criteria
Prototype behaviour under review:
Given an identity has optional Git attribution.
When Git loads that identity through a matching binding.
Then the included defaults apply unless ordinary Git precedence selects a stronger value.
DO NOT MERGE