fix(code): pre-configure worktree with GitHub noreply email#2342
Open
lost-particles wants to merge 1 commit into
Open
fix(code): pre-configure worktree with GitHub noreply email#2342lost-particles wants to merge 1 commit into
lost-particles wants to merge 1 commit into
Conversation
…rivate Closes PostHog#2156 When the user has "Keep my email address private" enabled on GitHub, commits authored with their real email get rejected on push with GH007 ("Your push would publish a private email address"). The agent previously had to recover from this error, and its behavior was inconsistent — sometimes it discovered the noreply email and amended the commit, sometimes it fell back to asking the user to either toggle the GitHub setting or paste the noreply address. This fix prevents the rejection from happening in the first place. After creating a worktree, we look up the authenticated GitHub user via `gh api user` and, when their public email is null (privacy enabled), set `user.name` / `user.email` on the worktree to the `<id>+<login>@users.noreply.github.com` form. The config is scoped to the worktree via `extensions.worktreeConfig`, so the user's main repo identity is untouched. The pre-configuration is best-effort: if `gh` is missing, unauthenticated, or the user has a public email, it's a no-op and the existing agent-side recovery still applies.
Contributor
Author
|
Hi maintainers, this PR addresses #2156. Whenever convenient, could someone from the team take a look and let me know if the approach (pre-configuring the worktree with the GitHub noreply identity at creation time) lines up with how you would like to solve this? Happy to iterate on scope, naming, or placement based on your feedback. Thanks! |
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+19
to
+100
| describe("computeNoreplyIdentity", () => { | ||
| it("returns noreply identity when GitHub user has email privacy enabled", () => { | ||
| expect( | ||
| computeNoreplyIdentity({ | ||
| id: 12345, | ||
| login: "octocat", | ||
| name: "Octo Cat", | ||
| email: null, | ||
| }), | ||
| ).toEqual({ | ||
| name: "Octo Cat", | ||
| email: "12345+octocat@users.noreply.github.com", | ||
| }); | ||
| }); | ||
|
|
||
| it("falls back to login when name is missing", () => { | ||
| expect( | ||
| computeNoreplyIdentity({ id: 12345, login: "octocat", email: null }), | ||
| ).toEqual({ | ||
| name: "octocat", | ||
| email: "12345+octocat@users.noreply.github.com", | ||
| }); | ||
| }); | ||
|
|
||
| it("returns null when GitHub user has a public email", () => { | ||
| expect( | ||
| computeNoreplyIdentity({ | ||
| id: 12345, | ||
| login: "octocat", | ||
| email: "octo@example.com", | ||
| }), | ||
| ).toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null when id or login is missing", () => { | ||
| expect( | ||
| computeNoreplyIdentity({ | ||
| id: 0, | ||
| login: "octocat", | ||
| email: null, | ||
| }), | ||
| ).toBeNull(); | ||
| expect( | ||
| computeNoreplyIdentity({ | ||
| id: 12345, | ||
| login: "", | ||
| email: null, | ||
| }), | ||
| ).toBeNull(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("fetchGitHubUserInfo", () => { | ||
| it("returns parsed user info when gh api user succeeds", async () => { | ||
| const runGh = vi | ||
| .fn() | ||
| .mockResolvedValue( | ||
| '{"id":12345,"login":"octocat","name":"Octo Cat","email":null}', | ||
| ); | ||
| const result = await fetchGitHubUserInfo({ runGh }); | ||
| expect(result).toEqual({ | ||
| id: 12345, | ||
| login: "octocat", | ||
| name: "Octo Cat", | ||
| email: null, | ||
| }); | ||
| expect(runGh).toHaveBeenCalledWith(["api", "user"]); | ||
| }); | ||
|
|
||
| it("returns null when gh fails (not installed / not authenticated)", async () => { | ||
| const runGh = vi.fn().mockRejectedValue(new Error("gh: command not found")); | ||
| expect(await fetchGitHubUserInfo({ runGh })).toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null when response is malformed JSON", async () => { | ||
| const runGh = vi.fn().mockResolvedValue("not json"); | ||
| expect(await fetchGitHubUserInfo({ runGh })).toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null when required fields are missing", async () => { | ||
| const runGh = vi.fn().mockResolvedValue('{"login":"octocat"}'); | ||
| expect(await fetchGitHubUserInfo({ runGh })).toBeNull(); |
Contributor
There was a problem hiding this comment.
The computeNoreplyIdentity and fetchGitHubUserInfo suites each have four independent it() cases that vary only in input/output — these are a natural fit for it.each tables. Parameterised tests are the convention in this codebase and keep the intent easier to scan as the number of cases grows.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/main/services/workspace/githubIdentity.test.ts
Line: 19-100
Comment:
**Prefer parameterised tests**
The `computeNoreplyIdentity` and `fetchGitHubUserInfo` suites each have four independent `it()` cases that vary only in input/output — these are a natural fit for `it.each` tables. Parameterised tests are the convention in this codebase and keep the intent easier to scan as the number of cases grows.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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
1 participant
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.
Closes #2156
Summary
git pushis rejected with GH007 ("Your push would publish a private email address"). The agent's recovery from this was inconsistent — sometimes it discovered the noreply email viagh/git history and amended the commit, sometimes it gave up and asked the user to either toggle the GitHub setting or paste the noreply.gh api user; when GitHub reports a public email, the worktree is left untouched.extensions.worktreeConfig+git config --worktree, so the user's main repo identity is never modified.Behavior
ghauthenticated<id>+<login>@users.noreply.github.com; push succeeds without agent recoverygh api userreturns a non-null email → no-op, worktree unchangedghmissing / not authenticatedThe pre-configuration is best-effort and wrapped in try/catch — any failure logs at debug level and does not break workspace creation.
Test plan
pnpm --filter code test src/main/services/workspace/githubIdentity.test.ts— 9 new tests cover the pure helper, mockedghfailure modes (not installed, malformed JSON, missing fields), and a real-git integration test verifying the noreply lands only on the worktree (main repo identity unchanged).pnpm --filter code test src/main— all 497 existing main-process tests still pass, including the archive integration test that exercises real worktree creation.pnpm --filter code exec biome checkon changed files — clean.gh auth logindone, create a new task workspace and rungit config user.emailin the resulting worktree — should be<id>+<login>@users.noreply.github.com. The main repo'sgit config --local user.emailshould be unchanged.user.emailshould fall through to the user's global config, not the noreply.Notes
fetchGitHubUserInfo(5s timeout ongh),computeNoreplyIdentity(pure),applyWorktreeIdentity.worktreeManager.create*call via a new privatepreconfigureNoreplyIdentityhelper.extensions.worktreeConfigis a benign opt-in (git 2.20+) that only enables per-worktree config storage; it doesn't alter any existing setting.