From 82681606d0a4c0fecf5487751812487abb489c4c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 28 Feb 2026 22:35:09 -0800 Subject: [PATCH] Remove git.renameBranch RPC and harden branch rename args - remove `git.renameBranch` from WS/contracts/native API surface and update server/web tests - use `git branch -m -- ...` in GitCore to safely separate branch rename arguments --- apps/server/src/git/Layers/GitCore.test.ts | 35 ++++++++++++++++++ apps/server/src/git/Layers/GitCore.ts | 4 +-- apps/server/src/git/Services/GitCore.ts | 12 +++++-- apps/server/src/wsServer.test.ts | 16 +-------- apps/server/src/wsServer.ts | 5 --- apps/web/src/wsNativeApi.ts | 1 - packages/contracts/src/git.ts | 42 ---------------------- packages/contracts/src/ipc.ts | 3 -- packages/contracts/src/ws.ts | 3 -- 9 files changed, 48 insertions(+), 73 deletions(-) diff --git a/apps/server/src/git/Layers/GitCore.test.ts b/apps/server/src/git/Layers/GitCore.test.ts index 19bd998a36a8f30d6c3d5cf726961e201b178e66..8e12cd0f187bd337c760a34f2baafe4f063122ff 100644 --- a/apps/server/src/git/Layers/GitCore.test.ts +++ b/apps/server/src/git/Layers/GitCore.test.ts @@ -913,6 +913,41 @@ it.layer(TestLayer)("git integration", (it) => { expect(renamed.branch).toBe("t3code/feat/session-2"); }), ); + + it.effect("uses '--' separator for branch rename arguments", () => + Effect.gen(function* () { + const tmp = yield* makeTmpDir(); + yield* initRepoWithCommit(tmp); + yield* createGitBranch({ cwd: tmp, branch: "feature/old-name" }); + yield* checkoutGitBranch({ cwd: tmp, branch: "feature/old-name" }); + + const realGitService = yield* GitService; + let renameArgs: ReadonlyArray | null = null; + const core = yield* makeIsolatedGitCore({ + execute: (input) => { + if (input.args[0] === "branch" && input.args[1] === "-m") { + renameArgs = [...input.args]; + } + return realGitService.execute(input); + }, + }); + + const renamed = yield* core.renameBranch({ + cwd: tmp, + oldBranch: "feature/old-name", + newBranch: "feature/new-name", + }); + + expect(renamed.branch).toBe("feature/new-name"); + expect(renameArgs).toEqual([ + "branch", + "-m", + "--", + "feature/old-name", + "feature/new-name", + ]); + }), + ); }); // ── createGitWorktree + removeGitWorktree ── diff --git a/apps/server/src/git/Layers/GitCore.ts b/apps/server/src/git/Layers/GitCore.ts index 06bb75afa8ee0fff54eddf963c3b3b20ac3c511b..25848ad5f3dfd44dd6eb67b3f3b299e4d665b1c9 100644 --- a/apps/server/src/git/Layers/GitCore.ts +++ b/apps/server/src/git/Layers/GitCore.ts @@ -282,7 +282,7 @@ const makeGitCore = Effect.gen(function* () { return yield* createGitCommandError( "GitCore.renameBranch", cwd, - ["branch", "-m", desiredBranch], + ["branch", "-m", "--", desiredBranch], `Could not find an available branch name for '${desiredBranch}'.`, ); }); @@ -1075,7 +1075,7 @@ const makeGitCore = Effect.gen(function* () { yield* executeGit( "GitCore.renameBranch", input.cwd, - ["branch", "-m", input.oldBranch, targetBranch], + ["branch", "-m", "--", input.oldBranch, targetBranch], { timeoutMs: 10_000, fallbackErrorMessage: "git branch rename failed", diff --git a/apps/server/src/git/Services/GitCore.ts b/apps/server/src/git/Services/GitCore.ts index 0c5d540075ae6e803d71550a58b72969a8ce6109..bc913d44ca2632958e28a343f90c9695728ab195 100644 --- a/apps/server/src/git/Services/GitCore.ts +++ b/apps/server/src/git/Services/GitCore.ts @@ -17,8 +17,6 @@ import type { GitListBranchesInput, GitListBranchesResult, GitPullResult, - GitRenameBranchInput, - GitRenameBranchResult, GitRemoveWorktreeInput, GitStatusInput, GitStatusResult, @@ -48,6 +46,16 @@ export interface GitRangeContext { diffPatch: string; } +export interface GitRenameBranchInput { + cwd: string; + oldBranch: string; + newBranch: string; +} + +export interface GitRenameBranchResult { + branch: string; +} + /** * GitCoreShape - Service API for low-level Git repository interactions. */ diff --git a/apps/server/src/wsServer.test.ts b/apps/server/src/wsServer.test.ts index bed3d5b90af403c765276b2a52e18db96684ba65..3f18e8929ecfd10d2676a401cfd44072760d87ad 100644 --- a/apps/server/src/wsServer.test.ts +++ b/apps/server/src/wsServer.test.ts @@ -371,7 +371,7 @@ describe("WebSocket Server", () => { gitManager?: GitManagerShape; gitCore?: Pick< GitCoreShape, - "listBranches" | "initRepo" | "pullCurrentBranch" | "renameBranch" + "listBranches" | "initRepo" | "pullCurrentBranch" >; terminalManager?: TerminalManagerShape; } = {}, @@ -1471,7 +1471,6 @@ describe("WebSocket Server", () => { }), ); const initRepo = vi.fn(() => Effect.void); - const renameBranch = vi.fn(() => Effect.succeed({ branch: "t3code/new-name" })); const pullCurrentBranch = vi.fn(() => Effect.fail( new GitCommandError({ @@ -1488,7 +1487,6 @@ describe("WebSocket Server", () => { gitCore: { listBranches, initRepo, - renameBranch, pullCurrentBranch, }, }); @@ -1513,18 +1511,6 @@ describe("WebSocket Server", () => { expect(pullResponse.error?.message).toContain("No upstream configured"); expect(pullCurrentBranch).toHaveBeenCalledWith("/repo/path"); - const renameResponse = await sendRequest(ws, WS_METHODS.gitRenameBranch, { - cwd: "/repo/path", - oldBranch: "t3code/tmp-456", - newBranch: "t3code/new-name", - }); - expect(renameResponse.error).toBeUndefined(); - expect(renameResponse.result).toEqual({ branch: "t3code/new-name" }); - expect(renameBranch).toHaveBeenCalledWith({ - cwd: "/repo/path", - oldBranch: "t3code/tmp-456", - newBranch: "t3code/new-name", - }); }); it("supports git.status over websocket", async () => { diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts index c0fa1e19de17e1bfbfbb9d3a45e8875f199cb9dd..5f4fb9d616a3d53c8ef6911c5a405167ec9a169b 100644 --- a/apps/server/src/wsServer.ts +++ b/apps/server/src/wsServer.ts @@ -572,11 +572,6 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< return yield* git.createWorktree(body); } - case WS_METHODS.gitRenameBranch: { - const body = stripRequestTag(request.body); - return yield* git.renameBranch(body); - } - case WS_METHODS.gitRemoveWorktree: { const body = stripRequestTag(request.body); return yield* git.removeWorktree(body); diff --git a/apps/web/src/wsNativeApi.ts b/apps/web/src/wsNativeApi.ts index a967ff6b8a963b381280ae73fd0dba285b7ea6da..f7ac04a9b07a6b4b10dc3214ea53b41b067a7e31 100644 --- a/apps/web/src/wsNativeApi.ts +++ b/apps/web/src/wsNativeApi.ts @@ -164,7 +164,6 @@ export function createWsNativeApi(): NativeApi { runStackedAction: (input) => transport.request(WS_METHODS.gitRunStackedAction, input), listBranches: (input) => transport.request(WS_METHODS.gitListBranches, input), createWorktree: (input) => transport.request(WS_METHODS.gitCreateWorktree, input), - renameBranch: (input) => transport.request(WS_METHODS.gitRenameBranch, input), removeWorktree: (input) => transport.request(WS_METHODS.gitRemoveWorktree, input), createBranch: (input) => transport.request(WS_METHODS.gitCreateBranch, input), checkout: (input) => transport.request(WS_METHODS.gitCheckout, input), diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index 4f9df0ccc1f90ab51d6cddfe2852a63756c25f2e..e0441366ef5aaae7ed893f26f7c5b14a4e97eda2 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -1,6 +1,5 @@ import { Schema } from "effect"; import { NonNegativeInt, PositiveInt, TrimmedNonEmptyString } from "./baseSchemas"; -import { ChatAttachment, PROVIDER_SEND_TURN_MAX_ATTACHMENTS } from "./orchestration"; const TrimmedNonEmptyStringSchema = TrimmedNonEmptyString; @@ -70,32 +69,6 @@ export const GitCreateWorktreeInput = Schema.Struct({ }); export type GitCreateWorktreeInput = typeof GitCreateWorktreeInput.Type; -export const GitGenerateBranchNameInput = Schema.Struct({ - cwd: TrimmedNonEmptyStringSchema, - message: TrimmedNonEmptyStringSchema, - attachments: Schema.optional( - Schema.Array(ChatAttachment).check(Schema.isMaxLength(PROVIDER_SEND_TURN_MAX_ATTACHMENTS)), - ), -}); -export type GitGenerateBranchNameInput = typeof GitGenerateBranchNameInput.Type; - -export const GitGenerateAndRenameBranchInput = Schema.Struct({ - cwd: TrimmedNonEmptyStringSchema, - oldBranch: TrimmedNonEmptyStringSchema, - message: TrimmedNonEmptyStringSchema, - attachments: Schema.optional( - Schema.Array(ChatAttachment).check(Schema.isMaxLength(PROVIDER_SEND_TURN_MAX_ATTACHMENTS)), - ), -}); -export type GitGenerateAndRenameBranchInput = typeof GitGenerateAndRenameBranchInput.Type; - -export const GitRenameBranchInput = Schema.Struct({ - cwd: TrimmedNonEmptyStringSchema, - oldBranch: TrimmedNonEmptyStringSchema, - newBranch: TrimmedNonEmptyStringSchema, -}); -export type GitRenameBranchInput = typeof GitRenameBranchInput.Type; - export const GitRemoveWorktreeInput = Schema.Struct({ cwd: TrimmedNonEmptyStringSchema, path: TrimmedNonEmptyStringSchema, @@ -164,21 +137,6 @@ export const GitCreateWorktreeResult = Schema.Struct({ }); export type GitCreateWorktreeResult = typeof GitCreateWorktreeResult.Type; -export const GitGenerateBranchNameResult = Schema.Struct({ - branch: Schema.NullOr(TrimmedNonEmptyStringSchema), -}); -export type GitGenerateBranchNameResult = typeof GitGenerateBranchNameResult.Type; - -export const GitGenerateAndRenameBranchResult = Schema.Struct({ - branch: Schema.NullOr(TrimmedNonEmptyStringSchema), -}); -export type GitGenerateAndRenameBranchResult = typeof GitGenerateAndRenameBranchResult.Type; - -export const GitRenameBranchResult = Schema.Struct({ - branch: TrimmedNonEmptyStringSchema, -}); -export type GitRenameBranchResult = typeof GitRenameBranchResult.Type; - export const GitRunStackedActionResult = Schema.Struct({ action: GitStackedAction, commit: Schema.Struct({ diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index 78bd34ac14db97f20cfe7ac0514ee19839ef4fd3..ce1c0305a54a918515205b55ba98a2fcfad11c0b 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -1,8 +1,6 @@ import type { GitCheckoutInput, GitCreateBranchInput, - GitRenameBranchInput, - GitRenameBranchResult, GitCreateWorktreeInput, GitCreateWorktreeResult, GitInitInput, @@ -76,7 +74,6 @@ export interface NativeApi { // Existing branch/worktree API listBranches: (input: GitListBranchesInput) => Promise; createWorktree: (input: GitCreateWorktreeInput) => Promise; - renameBranch: (input: GitRenameBranchInput) => Promise; removeWorktree: (input: GitRemoveWorktreeInput) => Promise; createBranch: (input: GitCreateBranchInput) => Promise; checkout: (input: GitCheckoutInput) => Promise; diff --git a/packages/contracts/src/ws.ts b/packages/contracts/src/ws.ts index 8f0ac0eaffcb3e1a2b4c07e4d83c09f3d35180e2..7f94615bdd47cc146697ac238a5b4590627b4c4f 100644 --- a/packages/contracts/src/ws.ts +++ b/packages/contracts/src/ws.ts @@ -16,7 +16,6 @@ import { GitInitInput, GitListBranchesInput, GitPullInput, - GitRenameBranchInput, GitRemoveWorktreeInput, GitRunStackedActionInput, GitStatusInput, @@ -51,7 +50,6 @@ export const WS_METHODS = { gitRunStackedAction: "git.runStackedAction", gitListBranches: "git.listBranches", gitCreateWorktree: "git.createWorktree", - gitRenameBranch: "git.renameBranch", gitRemoveWorktree: "git.removeWorktree", gitCreateBranch: "git.createBranch", gitCheckout: "git.checkout", @@ -113,7 +111,6 @@ export const WebSocketRequestBody = Schema.Union([ tagRequestBody(WS_METHODS.gitRunStackedAction, GitRunStackedActionInput), tagRequestBody(WS_METHODS.gitListBranches, GitListBranchesInput), tagRequestBody(WS_METHODS.gitCreateWorktree, GitCreateWorktreeInput), - tagRequestBody(WS_METHODS.gitRenameBranch, GitRenameBranchInput), tagRequestBody(WS_METHODS.gitRemoveWorktree, GitRemoveWorktreeInput), tagRequestBody(WS_METHODS.gitCreateBranch, GitCreateBranchInput), tagRequestBody(WS_METHODS.gitCheckout, GitCheckoutInput), -- 2.54.0