feat(code): harden branch linking during PR creation flow#1963
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/code/src/main/services/git/service.ts:632-638
**No test coverage for the new `linkBranch` call in `createPr`**
The test file updates `GitService` constructors to accept a `WorkspaceService` stub (`{} as WorkspaceService`), but no test verifies that `workspaceService.linkBranch` is called (with the right args) during `createPr`. This is the core behaviour change of the PR, and `createPr` currently has zero test coverage. If a proper mock were supplied (e.g. `{ linkBranch: vi.fn() } as unknown as WorkspaceService`), it would also prevent a silent `TypeError` if any future test exercises a code path through `createPr` with a `taskId`.
Reviews (1): Last reviewed commit: "feat(code): harden branch linking during..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (input.taskId) { | ||
| const linkedBranch = | ||
| input.branchName ?? (await getCurrentBranch(directoryPath)); | ||
| if (linkedBranch) { | ||
| this.workspaceService.linkBranch(input.taskId, linkedBranch, "user"); | ||
| } | ||
| } |
There was a problem hiding this comment.
linkBranch call in createPr
The test file updates GitService constructors to accept a WorkspaceService stub ({} as WorkspaceService), but no test verifies that workspaceService.linkBranch is called (with the right args) during createPr. This is the core behaviour change of the PR, and createPr currently has zero test coverage. If a proper mock were supplied (e.g. { linkBranch: vi.fn() } as unknown as WorkspaceService), it would also prevent a silent TypeError if any future test exercises a code path through createPr with a taskId.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/main/services/git/service.ts
Line: 632-638
Comment:
**No test coverage for the new `linkBranch` call in `createPr`**
The test file updates `GitService` constructors to accept a `WorkspaceService` stub (`{} as WorkspaceService`), but no test verifies that `workspaceService.linkBranch` is called (with the right args) during `createPr`. This is the core behaviour change of the PR, and `createPr` currently has zero test coverage. If a proper mock were supplied (e.g. `{ linkBranch: vi.fn() } as unknown as WorkspaceService`), it would also prevent a silent `TypeError` if any future test exercises a code path through `createPr` with a `taskId`.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Problem
The unified PR badge in the task header sometimes wouldn't render even though a PR existed — the simpler "View PR" + git options menu would show instead. Worse, it would only "fix itself" once the agent next edited a file.
The badge depends on
workspace.linkedBranchbeing set. Linking happened via a fire-and-forgetworkspace.linkBranch.mutate()call in the renderer (useGitInteraction.runCreatePr) right after the create-PR mutation returned. If the renderer reloaded mid-flow (e.g. Vite HMR during dev, or any IPC tear-down), that second mutation never landed on the main process, the link was silently lost (onlylog.warn'd), and the only recovery washandleAgentFileActivityre-linking on the next agent edit.#1947 made the happy path fast (subscription invalidation + optimistic
getPrUrlForBranchcache prime) but didn't address the durability of the link itself — when the fire-and-forget call is dropped, neither leg of #1947 has anything to act on.closes #1959
Changes
Move the
linkBranchcall into the main-processGitService.createPrflow so it's atomic with PR creation and survives renderer reloads.WorkspaceServiceintoGitService(no circular dep —WorkspaceServicedoesn't depend onGitService).workspaceService.linkBranch(taskId, branchName, "user"). Branch resolved asinput.branchName ?? getCurrentBranch(directoryPath).linkBranch.mutate()blocks inuseGitInteraction.runCreatePr. KeptinvalidateGitBranchQueriesand the optimisticgetPrUrlForBranchcache prime — those still provide the latency win from fix(code): invalidate queries on PR creation #1947, just no longer load-bearing for correctness.GitServicetest instantiations to pass aWorkspaceServicestub.How did you test this?
pnpm --filter code typecheckcleanpnpm --filter code test— all 982 tests passPublish to changelog?
no