Skip to content

feat(code): harden branch linking during PR creation flow #1963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adboio wants to merge 1 commit into
base: 04-30-chore_code_remove_diff_agent_actions_flag
Choose a base branch
from
Open

feat(code): harden branch linking during PR creation flow #1963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions apps/code/src/main/services/git/service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ vi.mock("../../utils/logger.js", () => ({
}));

import type { LlmGatewayService } from "../llm-gateway/service";
import type { WorkspaceService } from "../workspace/service";
import { GitService } from "./service";

describe("GitService.getPrChangedFiles", () => {
let service: GitService;

beforeEach(() => {
vi.clearAllMocks();
service = new GitService({} as LlmGatewayService);
service = new GitService({} as LlmGatewayService, {} as WorkspaceService);
});

it("flattens paginated GH API results and maps file statuses", async () => {
Expand Down Expand Up @@ -139,7 +140,7 @@ describe("GitService.getGhAuthToken", () => {

beforeEach(() => {
vi.clearAllMocks();
service = new GitService({} as LlmGatewayService);
service = new GitService({} as LlmGatewayService, {} as WorkspaceService);
});

it("returns the authenticated GitHub CLI token", async () => {
Expand Down Expand Up @@ -197,7 +198,7 @@ describe("GitService.getPrUrlForBranch", () => {

beforeEach(() => {
vi.clearAllMocks();
service = new GitService({} as LlmGatewayService);
service = new GitService({} as LlmGatewayService, {} as WorkspaceService);
});

it("returns the PR URL for a branch via gh pr list", async () => {
Expand Down
11 changes: 11 additions & 0 deletions apps/code/src/main/services/git/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { MAIN_TOKENS } from "../../di/tokens";
import { logger } from "../../utils/logger";
import { TypedEventEmitter } from "../../utils/typed-event-emitter";
import type { LlmGatewayService } from "../llm-gateway/service";
import type { WorkspaceService } from "../workspace/service";
import { CreatePrSaga } from "./create-pr-saga";
import type {
ChangedFile,
Expand Down Expand Up @@ -117,6 +118,8 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
constructor(
@inject(MAIN_TOKENS.LlmGatewayService)
private readonly llmGateway: LlmGatewayService,
@inject(MAIN_TOKENS.WorkspaceService)
private readonly workspaceService: WorkspaceService,
) {
super();
}
Expand Down Expand Up @@ -626,6 +629,14 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
includePrStatus: true,
});

if (input.taskId) {
const linkedBranch =
input.branchName ?? (await getCurrentBranch(directoryPath));
if (linkedBranch) {
this.workspaceService.linkBranch(input.taskId, linkedBranch, "user");
}
}
Comment on lines +632 to +638
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

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.


emitProgress(
"complete",
"Pull request created",
Expand Down
11 changes: 0 additions & 11 deletions apps/code/src/renderer/features/git-interaction/hooks/useGitInteraction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,17 +282,6 @@ export function useGitInteraction(
}
if (store.createPrNeedsBranch) {
invalidateGitBranchQueries(repoPath);
trpcClient.workspace.linkBranch
.mutate({ taskId, branchName: store.branchName.trim() })
.catch((err) =>
log.warn("Failed to link branch to task", { taskId, err }),
);
} else if (git.currentBranch) {
trpcClient.workspace.linkBranch
.mutate({ taskId, branchName: git.currentBranch })
.catch((err) =>
log.warn("Failed to link branch to task", { taskId, err }),
);
}

if (result.prUrl) {
Expand Down
Loading