From b1608f41adff7ac9670c5a762624e13984d820c8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 2 Mar 2026 09:58:52 -0800 Subject: [PATCH] Extract branch sanitization to shared contracts module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move sanitizeBranchFragment and sanitizeFeatureBranchName from three duplicate definitions (CodexTextGeneration, GitManager, GitActionsControl) into packages/contracts/src/git.ts. Remove the redundant double-sanitization of branch names between the TextGeneration and GitManager layers — CodexTextGeneration now owns sanitization of model output while GitManager trusts the result and only sanitizes the custom commit message path. Co-Authored-By: Claude Opus 4.6 --- .../src/git/Layers/CodexTextGeneration.ts | 30 ++-------------- apps/server/src/git/Layers/GitManager.ts | 33 ++--------------- .../src/components/GitActionsControl.logic.ts | 27 +++----------- packages/contracts/src/git.ts | 36 +++++++++++++++++++ 4 files changed, 47 insertions(+), 79 deletions(-) diff --git a/apps/server/src/git/Layers/CodexTextGeneration.ts b/apps/server/src/git/Layers/CodexTextGeneration.ts index 313ce8d995b9a53dab7dfad6cfc4bcc7cad7a702..4436c81def55e23e6ef8cb0d08087d82ab7351f1 100644 --- a/apps/server/src/git/Layers/CodexTextGeneration.ts +++ b/apps/server/src/git/Layers/CodexTextGeneration.ts @@ -3,6 +3,8 @@ import { randomUUID } from "node:crypto"; import { Effect, FileSystem, Layer, Option, Path, Schema, Stream } from "effect"; import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; +import { sanitizeBranchFragment, sanitizeFeatureBranchName } from "@t3tools/contracts"; + import { resolveAttachmentPath } from "../../attachmentStore.ts"; import { ServerConfig } from "../../config.ts"; import { TextGenerationError } from "../Errors.ts"; @@ -93,32 +95,6 @@ function sanitizePrTitle(raw: string): string { return "Update project changes"; } -function sanitizeBranchName(raw: string): string { - const normalized = raw - .trim() - .toLowerCase() - .replace(/['"`]/g, "") - .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); - - const branchFragment = normalized - .replace(/[^a-z0-9/_-]+/g, "-") - .replace(/\/+/g, "/") - .replace(/-+/g, "-") - .replace(/^[./_-]+|[./_-]+$/g, "") - .slice(0, 64) - .replace(/[./_-]+$/g, ""); - - return branchFragment.length > 0 ? branchFragment : "update"; -} - -function sanitizeFeatureBranchName(raw: string): string { - const sanitized = sanitizeBranchName(raw); - if (sanitized.includes("/")) { - return sanitized.startsWith("feature/") ? sanitized : `feature/${sanitized}`; - } - return `feature/${sanitized}`; -} - const makeCodexTextGeneration = Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -475,7 +451,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { }); return { - branch: sanitizeBranchName(generated.branch), + branch: sanitizeBranchFragment(generated.branch), } satisfies BranchNameGenerationResult; }); }; diff --git a/apps/server/src/git/Layers/GitManager.ts b/apps/server/src/git/Layers/GitManager.ts index 92d6fffcecdf57facbf1b42e8ae6899468a8d901..b7282b4055b69b02f3fdf4f96635be73b6f17897 100644 --- a/apps/server/src/git/Layers/GitManager.ts +++ b/apps/server/src/git/Layers/GitManager.ts @@ -1,6 +1,7 @@ import { randomUUID } from "node:crypto"; import { Effect, FileSystem, Layer, Path } from "effect"; +import { sanitizeFeatureBranchName } from "@t3tools/contracts"; import { GitManagerError } from "../Errors.ts"; import { GitManager, type GitManagerShape } from "../Services/GitManager.ts"; @@ -104,32 +105,6 @@ function sanitizeCommitMessage(generated: { }; } -function sanitizeBranchFragment(raw: string): string { - const normalized = raw - .trim() - .toLowerCase() - .replace(/['"`]/g, "") - .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); - - const branchFragment = normalized - .replace(/[^a-z0-9/_-]+/g, "-") - .replace(/\/+/g, "/") - .replace(/-+/g, "-") - .replace(/^[./_-]+|[./_-]+$/g, "") - .slice(0, 64) - .replace(/[./_-]+$/g, ""); - - return branchFragment.length > 0 ? branchFragment : "update"; -} - -function sanitizeFeatureBranchName(raw: string): string { - const sanitized = sanitizeBranchFragment(raw); - if (sanitized.includes("/")) { - return sanitized.startsWith("feature/") ? sanitized : `feature/${sanitized}`; - } - return `feature/${sanitized}`; -} - function formatCommitMessage(subject: string, body: string): string { const trimmedBody = body.trim(); if (trimmedBody.length === 0) { @@ -324,16 +299,14 @@ export const makeGitManager = Effect.gen(function* () { branch: input.branch, stagedSummary: limitContext(context.stagedSummary, 8_000), stagedPatch: limitContext(context.stagedPatch, 50_000), - ...(input.includeBranch !== undefined ? { includeBranch: input.includeBranch } : {}), + ...(input.includeBranch ? { includeBranch: true } : {}), }) .pipe(Effect.map((result) => sanitizeCommitMessage(result))); return { subject: generated.subject, body: generated.body, - ...(generated.branch !== undefined - ? { branch: sanitizeFeatureBranchName(generated.branch) } - : {}), + ...(generated.branch !== undefined ? { branch: generated.branch } : {}), commitMessage: formatCommitMessage(generated.subject, generated.body), }; }); diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts index 23cd63e6284fe0975ae44ad913609379689fe4b5..bbea434f787df5829ae6d95eb3f0f520a07335cf 100644 --- a/apps/web/src/components/GitActionsControl.logic.ts +++ b/apps/web/src/components/GitActionsControl.logic.ts @@ -1,7 +1,8 @@ -import type { - GitRunStackedActionResult, - GitStackedAction, - GitStatusResult, +import { + type GitRunStackedActionResult, + type GitStackedAction, + type GitStatusResult, + sanitizeBranchFragment, } from "@t3tools/contracts"; export type GitActionIconName = "commit" | "push" | "pr"; @@ -321,24 +322,6 @@ export function resolveDefaultBranchActionDialogCopy(input: { }; } -function sanitizeBranchFragment(raw: string): string { - const normalized = raw - .trim() - .toLowerCase() - .replace(/['"`]/g, "") - .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); - - const branchFragment = normalized - .replace(/[^a-z0-9/_-]+/g, "-") - .replace(/\/+/g, "/") - .replace(/-+/g, "-") - .replace(/^[./_-]+|[./_-]+$/g, "") - .slice(0, 64) - .replace(/[./_-]+$/g, ""); - - return branchFragment.length > 0 ? branchFragment : "update"; -} - export function resolveAutoFeatureBranchName( existingBranchNames: readonly string[], preferredBranch?: string, diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index 12bd739dcf955873f37173b75656c4926e73a167..7e7708b74b5b4aec090042441f855c28ee2c732d 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -179,3 +179,39 @@ export const GitPullResult = Schema.Struct({ upstreamBranch: TrimmedNonEmptyStringSchema.pipe(Schema.NullOr), }); export type GitPullResult = typeof GitPullResult.Type; + +// Branch name utilities + +/** + * Sanitize an arbitrary string into a valid, lowercase git branch fragment. + * Strips quotes, collapses separators, limits to 64 chars. + */ +export function sanitizeBranchFragment(raw: string): string { + const normalized = raw + .trim() + .toLowerCase() + .replace(/['"`]/g, "") + .replace(/^[./\s_-]+|[./\s_-]+$/g, ""); + + const branchFragment = normalized + .replace(/[^a-z0-9/_-]+/g, "-") + .replace(/\/+/g, "/") + .replace(/-+/g, "-") + .replace(/^[./_-]+|[./_-]+$/g, "") + .slice(0, 64) + .replace(/[./_-]+$/g, ""); + + return branchFragment.length > 0 ? branchFragment : "update"; +} + +/** + * Sanitize a string into a `feature/…` branch name. + * Preserves an existing `feature/` prefix or slash-separated namespace. + */ +export function sanitizeFeatureBranchName(raw: string): string { + const sanitized = sanitizeBranchFragment(raw); + if (sanitized.includes("/")) { + return sanitized.startsWith("feature/") ? sanitized : `feature/${sanitized}`; + } + return `feature/${sanitized}`; +} -- 2.54.0